[haiku-commits] Re: haiku: hrev44039 - src/kits/interface headers/os/interface src/preferences/keymap

  • From: John Scipione <jscipione@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 17 Apr 2012 16:23:04 -0400

On Mon, Apr 16, 2012 at 3:32 AM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>wrote:

> On 16.04.2012 08:30, jscipione@xxxxxxxxx wrote:
>
>>   * Overloaded methods with radium parameters are not virtual right
>
>     now so as to not break vtables. Added /*virtual*/ before each
>>     method that should be made virtual in ControlLook.h
>>
>
> When you fix that, make sure you'll add quite a bunch of reserved virtual
> slots for the next time.
>
>> +       // reset clipping constraints
>>        view->ConstrainClippingRegion(**NULL);
>>  }
>>
>
> I know it's not your code, but is it a good idea to change the views
> clipping region outside of the view state push/pop?
>

As a bit of an olive branch here, I do agree with and appreciate your other
comments, we should add reserved virtual method slots and we should be
using view->PushState() and view->PopState() to preserve the clipping
region and other parameters of the view. I tried to get that to work in
fact but I ran into some issues and I didn't want to mess with the
functions outside of buttons and menu frames too much, even then I wanted
to remain conservative in my change and not try to alter the overall design
of ControlLook too much. For instance I made a mistake accidentally
deleting a line from the slider drawing code which I had to fixup.

Adding virtual method slots will force us to recompile all apps that use
ControlLook (which is most of them). Perhaps R1/Alpha4 is a good
opportunity to make this kind of change since we'll probably want to
recompile apps for that release anyway.

I realize that it takes a lot of time to read through commit logs and I
appreciate that you are not afraid to call out problems when you see them.
Many of the comments you make have led me to make alterations, style or
otherwise that are better for everyone. Sometimes I don't even realize that
I am doing something improper for instance I updated the Deskbar
preferences to use layout-aware spacing because of a comment on an earlier
commit.

I agree that Apple has gone a bit far emulating real-world objects at times
like changing their address book app to look like a physical address
book. As far as these kind of changes going in over your dead body, I guess
you must be dead because as I've pointed out there are a bunch of them
already. And we should probably not include any theme options, but we can
make wholesale theme changes where appropriate, no options, but changes to
the theme are okay. Otherwise the look will never progress.

I also tend to heed your advice most of the time even when I disagree with
it. In this case I figured it was worth defending from the reactions I got
from people pleading me to keep the change.

John Scipione

Other related posts: