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