Re: [Gnustep-cvs] r34174 - in /libs/gui/trunk: ChangeLog Headers/Additions/GNUstepGUI/GSTheme.h Source/GSTheme.m Source/GSThemeDrawing.m Source/NSMenuItemCell.m

[Fred Kiefer]

On 22.11.2011 20:03, Eric Wasylishen wrote:

On 2011-11-22, at 8:17 AM, Quentin Mathé wrote:

And one final point. You us the method call [path setLineWidth:
0.0]; This isn't as well defined as it may seem. Most people think
that this will select the smallest possible line width. At least
with our current code this may not be true for the cairo backend.
There is a long and ongoing discussion about zero line widths on
the cairo mailing list.

ok, I wasn't aware of this issue. Which width should I use to get
the thinnest possible line or something close? 0.5 or 0.1 for
example?


I added a fix to the cairo backend a few months ago which will
interpret 0 as a thin line (I think previously I broke a game which
was relying on that behaviour.)


I didn't notice that change at the time, or forgot about it. In your 
Changelog entry you state that Quartz draws nothing for a line width of 
zero. Is this only the low level or do the higher level drawing 
operations also ignore zero line width drawing?



However, I recommend drawing a filled rectangle instead of using
lines. In general, anywhere you have lines in a user interface, they
should probably be filled rectangles. Filled rectangles scale up in a
predictable way and are easy to pixel-align (just make the origin and
size integers.)


Yes, filling rectangle is more efficient, but you don't want to compute 
the corresponding rectangles all the time. On the other hand, the user 
interface elements we draw are so simple, we could easily come up with 
the rectangles for them.


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev

Re: [Gnustep-cvs] r34174 - in /libs/gui/trunk: ChangeLog Headers/Additions/GNUstepGUI/GSTheme.h Source/GSTheme.m Source/GSThemeDrawing.m Source/NSMenuItemCell.m

[Quentin Mathé]

Hi Fred,

Sorry for the late reply.

On Thu, Nov 17, 2011 at 10:12 AM, Fred Kiefer fredkie...@gmx.de wrote:

 On 16.11.2011 11:32, Quentin Math wrote:

 Author: qmathe
 Date: Wed Nov 16 11:32:15 2011
 New Revision: 34174

 URL: 
 http://svn.gna.org/viewcvs/**gnustep?rev=34174view=revhttp://svn.gna.org/viewcvs/gnustep?rev=34174view=rev
 Log:
 Improved the menu theming to support some common menu look variations.

 Fixed bug #34792 too.

 Modified:
 libs/gui/trunk/ChangeLog
 libs/gui/trunk/Headers/**Additions/GNUstepGUI/GSTheme.h
 libs/gui/trunk/Source/GSTheme.**m
 libs/gui/trunk/Source/**GSThemeDrawing.m
 libs/gui/trunk/Source/**NSMenuItemCell.m


 Hi Quentin,

 I like the changes you made for menu theming. And would like to use these
 as a starting point for a bit of a general discussion on theming.


Sounds like a good idea.


 In this change you introduced a few new colour methods on GSTheme and
 commented on another place in NSMenuItemCell.m
 // TODO: Make the color lookup simpler.

 Now the way we make the colour lookup in GSTheme was decided by Richard
 some time ago. He wanted to have basic lookup methods for all the different
 theme operations to be similar and came up with the current interface for
 colour lookup. It is not that I like this interface that much, but we
 should try to stay consistent. For this reason I think we should use this
 as the official interface for GSTheme instead of introducing new methods as
 we go. I know I did so myself for example for toolbar colours.


I must admit I don't know how to edit the color lists easily. Thematic only
knows about some precise colors but cannot edit arbitrary color lists.
For example, how can I make +[NSColor controlBackgroundColor] uses
+[NSColor greenColor] ? Can I do that with the built-in color picker ?
We also seem to have no tools to convert binary encoded NSColorList into
the ASCII format and vice-versa.

About the current API… The GSTheme color lookup API includes a method
-colorNamed:state:, but the state is almost never used based on a quick
grep over the Gui source code (only once in NSMenuItemCell.m). So I would
remove the last argument and encode the state within the color name.
NSColor API names the color like that, this approach would avoid to
introduce two color naming schemes (GTheme vs NSColor) for the themable
colors.

The state arguments makes sense with the tile lookup methods, but for the
themable colors I'm not convinced.

Having color methods declared in GSTheme (such as the toolbar one) as
syntactic sugar over -colorNamed: doesn't go against the current color
lookup. It might even be a good way to clearly document the colors that can
be customized.


 I like your new themeControlState method. We should use this in all the
 other places where it is useful, for example in the method backgroundColor.


Yes, it could even be used in other classes such as -[NSButtonCell
drawBezelWithFrame:inView:].


 This is actually the only place where you use one of your new colour
 methods outside of the GSTheme class, maybe we should just change the
 lookup key here to menuItemBackgroundColor and adjust all the themes?


For -[NSMenuItemCell backgroundColor], I would suggest to remove:

  color = [[GSTheme theme] colorNamed: @NSMenuItem state: state];
  if (color == nil)

and use either:

  if ((state == GSThemeHighlightedState) || (state ==
GSThemeSelectedState))
{
  color = [NSColor selectedMenuItemColor];
}
  else
{
  color = [[GSTheme theme] menuItemBackgroundColor];
}
or

  if ((state == GSThemeHighlightedState) || (state ==
GSThemeSelectedState))
{
  color = [NSColor selectedMenuItemColor];
}
  else
{
  color = [[GSTheme theme] colorNamed: @menuItemBackgroundColor];
}

Should I use this last solution?

Then there is this call in your code:
 NSInterfaceStyleForKey(@**NSMenuInterfaceStyle, nil);
 We have plenty of similar calls all over the place, still I think this is
 wrong. When ever possible we should pass in a responder as the second
 argument of this function call. Every view is allowed to have its specific
 interface style and at least the standard drawing code should respect this.
 Themes may handle this differently or just ignore that value.


I'll remove this call in -menuSeparatorColor, it isn't needed now that the
Windows theme uses native menus as Gregory pointed it out in the bug report
#34792.

I agree about passing a responder every time to the GSTheme methods where
it makes sense, and limiting the number of places where we use
NSInterfaceStyleForKey(). Most uses of NSInterfaceStyleForKey() should be
in GSTheme subclasses I think.


 And one final point. You us the method call [path setLineWidth: 0.0]; This
 isn't as well defined as it may seem. Most people think that this will
 select the smallest possible line width. At least with our current code
 this may not be true for the cairo backend. There is a long and ongoing
 discussion about zero line widths on the cairo mailing 

Re: [Gnustep-cvs] r34174 - in /libs/gui/trunk: ChangeLog Headers/Additions/GNUstepGUI/GSTheme.h Source/GSTheme.m Source/GSThemeDrawing.m Source/NSMenuItemCell.m

[Eric Wasylishen]

Hi Fred and Quentin,

On 2011-11-22, at 8:17 AM, Quentin Mathé wrote:
 
 Hi Quentin,
 
 I like the changes you made for menu theming. And would like to use these as 
 a starting point for a bit of a general discussion on theming.
 
 Sounds like a good idea. 

Yes, agreed.

 In this change you introduced a few new colour methods on GSTheme and 
 commented on another place in NSMenuItemCell.m
 // TODO: Make the color lookup simpler.
 
 Now the way we make the colour lookup in GSTheme was decided by Richard some 
 time ago. He wanted to have basic lookup methods for all the different theme 
 operations to be similar and came up with the current interface for colour 
 lookup. It is not that I like this interface that much, but we should try to 
 stay consistent. For this reason I think we should use this as the official 
 interface for GSTheme instead of introducing new methods as we go. I know I 
 did so myself for example for toolbar colours.
 
 I must admit I don't know how to edit the color lists easily. Thematic only 
 knows about some precise colors but cannot edit arbitrary color lists. 
 For example, how can I make +[NSColor controlBackgroundColor] uses +[NSColor 
 greenColor] ? Can I do that with the built-in color picker ?
 We also seem to have no tools to convert binary encoded NSColorList into the 
 ASCII format and vice-versa.

 About the current API… The GSTheme color lookup API includes a method 
 -colorNamed:state:, but the state is almost never used based on a quick grep 
 over the Gui source code (only once in NSMenuItemCell.m). So I would remove 
 the last argument and encode the state within the color name. NSColor API 
 names the color like that, this approach would avoid to introduce two color 
 naming schemes (GTheme vs NSColor) for the themable colors.
 
 The state arguments makes sense with the tile lookup methods, but for the 
 themable colors I'm not convinced.
 
 Having color methods declared in GSTheme (such as the toolbar one) as 
 syntactic sugar over -colorNamed: doesn't go against the current color 
 lookup. It might even be a good way to clearly document the colors that can 
 be customized.
  
 I like your new themeControlState method. We should use this in all the other 
 places where it is useful, for example in the method backgroundColor.
 
 Yes, it could even be used in other classes such as -[NSButtonCell 
 drawBezelWithFrame:inView:].
  
 This is actually the only place where you use one of your new colour methods 
 outside of the GSTheme class, maybe we should just change the lookup key here 
 to menuItemBackgroundColor and adjust all the themes?
 
 For -[NSMenuItemCell backgroundColor], I would suggest to remove:
 
   color = [[GSTheme theme] colorNamed: @NSMenuItem state: state];
   if (color == nil)
 
 and use either:
 
   if ((state == GSThemeHighlightedState) || (state == 
 GSThemeSelectedState))
   {
 color = [NSColor selectedMenuItemColor];
   }
   else
   {
 color = [[GSTheme theme] menuItemBackgroundColor];
   }
 or
 
   if ((state == GSThemeHighlightedState) || (state == 
 GSThemeSelectedState))
   {
 color = [NSColor selectedMenuItemColor];
   }
   else
   {
 color = [[GSTheme theme] colorNamed: @menuItemBackgroundColor];
   }
 
 Should I use this last solution?
 
 Then there is this call in your code:
 NSInterfaceStyleForKey(@NSMenuInterfaceStyle, nil);
 We have plenty of similar calls all over the place, still I think this is 
 wrong. When ever possible we should pass in a responder as the second 
 argument of this function call. Every view is allowed to have its specific 
 interface style and at least the standard drawing code should respect this. 
 Themes may handle this differently or just ignore that value.
 
 I'll remove this call in -menuSeparatorColor, it isn't needed now that the 
 Windows theme uses native menus as Gregory pointed it out in the bug report 
 #34792.

 I agree about passing a responder every time to the GSTheme methods where it 
 makes sense, and limiting the number of places where we use 
 NSInterfaceStyleForKey(). Most uses of NSInterfaceStyleForKey() should be in 
 GSTheme subclasses I think.
  
 And one final point. You us the method call [path setLineWidth: 0.0]; This 
 isn't as well defined as it may seem. Most people think that this will select 
 the smallest possible line width. At least with our current code this may not 
 be true for the cairo backend. There is a long and ongoing discussion about 
 zero line widths on the cairo mailing list.
 
 ok, I wasn't aware of this issue. Which width should I use to get the 
 thinnest possible line or something close? 0.5 or 0.1 for example?


I added a fix to the cairo backend a few months ago which will interpret 0 as a 
thin line (I think previously I broke a game which was relying on that 
behaviour.)

However, I recommend drawing a filled rectangle instead of using lines. In 
general, anywhere you 

Re: [Gnustep-cvs] r34174 - in /libs/gui/trunk: ChangeLog Headers/Additions/GNUstepGUI/GSTheme.h Source/GSTheme.m Source/GSThemeDrawing.m Source/NSMenuItemCell.m

[Fred Kiefer]

On 16.11.2011 11:32, Quentin Math wrote:

Author: qmathe
Date: Wed Nov 16 11:32:15 2011
New Revision: 34174

URL: http://svn.gna.org/viewcvs/gnustep?rev=34174view=rev
Log:
Improved the menu theming to support some common menu look variations.

Fixed bug #34792 too.

Modified:
 libs/gui/trunk/ChangeLog
 libs/gui/trunk/Headers/Additions/GNUstepGUI/GSTheme.h
 libs/gui/trunk/Source/GSTheme.m
 libs/gui/trunk/Source/GSThemeDrawing.m
 libs/gui/trunk/Source/NSMenuItemCell.m


Hi Quentin,

I like the changes you made for menu theming. And would like to use 
these as a starting point for a bit of a general discussion on theming.


In this change you introduced a few new colour methods on GSTheme and 
commented on another place in NSMenuItemCell.m

// TODO: Make the color lookup simpler.

Now the way we make the colour lookup in GSTheme was decided by Richard 
some time ago. He wanted to have basic lookup methods for all the 
different theme operations to be similar and came up with the current 
interface for colour lookup. It is not that I like this interface that 
much, but we should try to stay consistent. For this reason I think we 
should use this as the official interface for GSTheme instead of 
introducing new methods as we go. I know I did so myself for example for 
toolbar colours.


I like your new themeControlState method. We should use this in all the 
other places where it is useful, for example in the method 
backgroundColor. This is actually the only place where you use one of 
your new colour methods outside of the GSTheme class, maybe we should 
just change the lookup key here to menuItemBackgroundColor and adjust 
all the themes?


Then there is this call in your code:
NSInterfaceStyleForKey(@NSMenuInterfaceStyle, nil);
We have plenty of similar calls all over the place, still I think this 
is wrong. When ever possible we should pass in a responder as the second 
argument of this function call. Every view is allowed to have its 
specific interface style and at least the standard drawing code should 
respect this. Themes may handle this differently or just ignore that value.


And one final point. You us the method call [path setLineWidth: 0.0]; 
This isn't as well defined as it may seem. Most people think that this 
will select the smallest possible line width. At least with our current 
code this may not be true for the cairo backend. There is a long and 
ongoing discussion about zero line widths on the cairo mailing list.


Cheers
Fred

___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev