Re: Arrow of NSMenu

2013-04-08 Thread Riccardo Mottola

Hi Eric,

thanks for looking at this again.

On 04/06/13 21:51, Eric Wasylishen wrote:


I committed a more radical redesign that is much cleaner, I think.
Pasting my changelog comment:

I removed the step in theme activation where we call
+[NSImage _setImagePath:name:] on each image in the theme, and instead
modified +[NSImage _pathForImageNamed:] to also search the theme images
directory.

When a GSTheme activates now, it only calls +[NSImage 
_reloadCachedImages]
which checks all NSImage cached by name and reloads any whose path has
changed.
If I modify manually my theme by adding the Image in the ThemeImages 
resources, it works now fine, i t loads and unloads perfectly and the 
menu gets updated. Very nice!


When i try to change the theme in Thematic I get a crash, but I will 
open a separate thread, I still am unsure if it is a Thematic bug (but I 
don't see how this is image is treated differently there) or a problem 
with the new change, for sure it did nto happen before (but it did not 
work either).


My only worry is whether this will break the GTK or windows themes. IIRC they 
replace the NSImage class and override +imageNamed:, so I think they'll still 
work.
The Windows theme was already broken since months and it did not load 
images at all, I will test it though soon.


Riccardo

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


Re: Arrow of NSMenu

2013-04-06 Thread Eric Wasylishen
Hi Riccardo,

I committed a fix in r36474. What I ended up doing is, when a GSTheme 
activates, it takes the image name - path dictionary of images in the theme, 
and expands it by applying all of the nsmappings.strings mappings.

So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll 
produce a dictionary like:
{
common_3DArrowRight : path/to/common_3DArrowRight.tiff,
NSMenuArrow : path/to/common_3DArrowRight.tiff,
}

This expanded set of images is then applied to the app state using +[NSImage 
_setImagePath:name:], and the same expanded set is unregistered when the theme 
deactivates.

Hope this works for you, and the behaviour sounds sensible.

Cheers.
Eric

On 2013-04-05, at 5:25 AM, Riccardo Mottola riccardo.mott...@libero.it wrote:

 Hi all,
 
 On 03/28/13 16:40, Eric Wasylishen wrote:
 
 Hey Riccardo, check the nsmappings.strings file in Images. I think that maps 
 nsmenuarrow to one of the common_ images.
 
 back to the original problem, which is different from what German supposed.
 
 Let's remember that we have a mapping
 
 NSMenuArrow = common_3DArrowRight;
 
 
 Leaving out Thematic for a moment, I found that placing inside the Theme 
 Images a an image named
 
 common_3DArrowRight.tif
 
 doesn't work, while putting one called
 
 NSMenuArrow.tif
 
 works fine and the image gets loaded even dynamically when changing the theme.
 
 
 Riccardo


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


Re: Arrow of NSMenu

2013-04-06 Thread Fred Kiefer

Hi Eric,

I don't like this change very much and will try explain why. This does 
not mean that I doubt the technical correctness of this patch. I just 
think we should try to find a better solution.


- First off, I don't really see the issue here. This may be because I 
don't use themes. But can somebody please explain what would be the 
problem with using the official names for images in themes? Riccardo 
already stated that things would work when using the name NSMenuArrow.


- The big doubt I am having with the change is that now GSTheme has to 
know about that name mapping which was internal to NSImage up to now.
A solution where similar code would be used inside the NSImage method 
_setImagePath:name: seems a lot cleaner to me. If we build up that 
reverse map you are using, all the necessary information should be 
available in that method. I think this would belong into the else case 
of the if (image != nil) test you introduced.


- Another way to get rid of the problem would be to completely remove 
the name mapping from NSImage. I am a bit reluctant to propose this. 
That mechanism has been around for a very long time and it allows us to 
have clearer names. But with the theme code in place this mechanism 
isn't needed that much any more.


Cheers
Fred

On 06.04.2013 10:06, Eric Wasylishen wrote:

Hi Riccardo,

I committed a fix in r36474. What I ended up doing is, when a GSTheme activates, it takes 
the image name - path dictionary of images in the theme, and expands it by 
applying all of the nsmappings.strings mappings.

So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll 
produce a dictionary like:
{
common_3DArrowRight : path/to/common_3DArrowRight.tiff,
NSMenuArrow : path/to/common_3DArrowRight.tiff,
}

This expanded set of images is then applied to the app state using +[NSImage 
_setImagePath:name:], and the same expanded set is unregistered when the theme 
deactivates.

Hope this works for you, and the behaviour sounds sensible.

Cheers.
Eric

On 2013-04-05, at 5:25 AM, Riccardo Mottola riccardo.mott...@libero.it wrote:


Hi all,

On 03/28/13 16:40, Eric Wasylishen wrote:


Hey Riccardo, check the nsmappings.strings file in Images. I think that maps 
nsmenuarrow to one of the common_ images.


back to the original problem, which is different from what German supposed.

Let's remember that we have a mapping

NSMenuArrow = common_3DArrowRight;


Leaving out Thematic for a moment, I found that placing inside the Theme Images 
a an image named

common_3DArrowRight.tif

doesn't work, while putting one called

NSMenuArrow.tif

works fine and the image gets loaded even dynamically when changing the theme.



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


Re: Arrow of NSMenu

2013-04-06 Thread Riccardo Mottola

Hi,

I am a little in doubt here, I think Fred has some valid points.

Fred Kiefer wrote:

Hi Eric,

I don't like this change very much and will try explain why. This does 
not mean that I doubt the technical correctness of this patch. I just 
think we should try to find a better solution.


- First off, I don't really see the issue here. This may be because I 
don't use themes. But can somebody please explain what would be the 
problem with using the official names for images in themes? Riccardo 
already stated that things would work when using the name NSMenuArrow.

Well, but if we have a remapping it should work. IE. our actual file is
common_3DArrowRight.tif in the standard theme, why shouldn't that be 
true also in the theme-supplied images?


- The big doubt I am having with the change is that now GSTheme has to 
know about that name mapping which was internal to NSImage up to now.
A solution where similar code would be used inside the NSImage method 
_setImagePath:name: seems a lot cleaner to me. If we build up that 
reverse map you are using, all the necessary information should be 
available in that method. I think this would belong into the else case 
of the if (image != nil) test you introduced.


- Another way to get rid of the problem would be to completely remove 
the name mapping from NSImage. I am a bit reluctant to propose this. 
That mechanism has been around for a very long time and it allows us 
to have clearer names. But with the theme code in place this mechanism 
isn't needed that much any more.
Well, mapping works with the default theme. I too don't like that 
GSTheme shouldn't know about the remapping.


You ask for NSArrow, the mapping converts it to another name, now this 
other named image could be in the default theme or in a supplied theme. 
I actually do not understand where the mechamism breaks.


Riccardo

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


Re: Arrow of NSMenu

2013-04-06 Thread Eric Wasylishen
Hi Fred,

On 2013-04-06, at 9:03 AM, Fred Kiefer fredkie...@gmx.de wrote:

 Hi Eric,
 
 I don't like this change very much and will try explain why. This does not 
 mean that I doubt the technical correctness of this patch. I just think we 
 should try to find a better solution.

Agreed, it's ugly.

 - First off, I don't really see the issue here. This may be because I don't 
 use themes. But can somebody please explain what would be the problem with 
 using the official names for images in themes? Riccardo already stated that 
 things would work when using the name NSMenuArrow.

If we don't use the nsmappings.strings for themes, themes may have to provide a 
lot of duplicate files (e..g common_3DArrowRight.tiff, NSMenuArrow.tiff) with 
the same contents. Not a huge problem… but it's ugly to have different image 
lookup logic for images inside themes and other images.

 - The big doubt I am having with the change is that now GSTheme has to know 
 about that name mapping which was internal to NSImage up to now.
 A solution where similar code would be used inside the NSImage method 
 _setImagePath:name: seems a lot cleaner to me. If we build up that reverse 
 map you are using, all the necessary information should be available in that 
 method. I think this would belong into the else case of the if (image != nil) 
 test you introduced.

I considered that - so +[NSImage _setImagePath:name:] for common_3DArrowRight 
would also set the path for NSMenuArrow and anything else that maps to 
common_3DArrowRight.

There is a corner case with that; if the theme provides both NSMenuArrow and 
common_3DArrowRight, the image used for NSMenuArrow depends on which call to 
+[NSImage _setImagePath:name:] is made first.

 - Another way to get rid of the problem would be to completely remove the 
 name mapping from NSImage. I am a bit reluctant to propose this. That 
 mechanism has been around for a very long time and it allows us to have 
 clearer names. But with the theme code in place this mechanism isn't needed 
 that much any more.

I committed a more radical redesign that is much cleaner, I think.
Pasting my changelog comment:

I removed the step in theme activation where we call
+[NSImage _setImagePath:name:] on each image in the theme, and instead
modified +[NSImage _pathForImageNamed:] to also search the theme images
directory.

When a GSTheme activates now, it only calls +[NSImage 
_reloadCachedImages]
which checks all NSImage cached by name and reloads any whose path has
changed.

My only worry is whether this will break the GTK or windows themes. IIRC they 
replace the NSImage class and override +imageNamed:, so I think they'll still 
work.

Eric

 Cheers
 Fred
 
 On 06.04.2013 10:06, Eric Wasylishen wrote:
 Hi Riccardo,
 
 I committed a fix in r36474. What I ended up doing is, when a GSTheme 
 activates, it takes the image name - path dictionary of images in the 
 theme, and expands it by applying all of the nsmappings.strings mappings.
 
 So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll 
 produce a dictionary like:
 {
  common_3DArrowRight : path/to/common_3DArrowRight.tiff,
  NSMenuArrow : path/to/common_3DArrowRight.tiff,
 }
 
 This expanded set of images is then applied to the app state using +[NSImage 
 _setImagePath:name:], and the same expanded set is unregistered when the 
 theme deactivates.
 
 Hope this works for you, and the behaviour sounds sensible.
 
 Cheers.
 Eric
 
 On 2013-04-05, at 5:25 AM, Riccardo Mottola riccardo.mott...@libero.it 
 wrote:
 
 Hi all,
 
 On 03/28/13 16:40, Eric Wasylishen wrote:
 
 Hey Riccardo, check the nsmappings.strings file in Images. I think that 
 maps nsmenuarrow to one of the common_ images.
 
 back to the original problem, which is different from what German supposed.
 
 Let's remember that we have a mapping
 
 NSMenuArrow = common_3DArrowRight;
 
 
 Leaving out Thematic for a moment, I found that placing inside the Theme 
 Images a an image named
 
 common_3DArrowRight.tif
 
 doesn't work, while putting one called
 
 NSMenuArrow.tif
 
 works fine and the image gets loaded even dynamically when changing the 
 theme.
 
 
 ___
 Gnustep-dev mailing list
 Gnustep-dev@gnu.org
 https://lists.gnu.org/mailman/listinfo/gnustep-dev


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


Re: Arrow of NSMenu

2013-04-06 Thread Fred Kiefer
Hi Eric,

thank you! This new change looks a lot cleaner and leaves all the mapping logic 
inside of NSImage. I think it will be a lot easier to maintain.

Fred

On the road

Am 06.04.2013 um 21:51 schrieb Eric Wasylishen ewasylis...@gmail.com:

 Hi Fred,
 
 On 2013-04-06, at 9:03 AM, Fred Kiefer fredkie...@gmx.de wrote:
 
 Hi Eric,
 
 I don't like this change very much and will try explain why. This does not 
 mean that I doubt the technical correctness of this patch. I just think we 
 should try to find a better solution.
 
 Agreed, it's ugly.
 
 - First off, I don't really see the issue here. This may be because I don't 
 use themes. But can somebody please explain what would be the problem with 
 using the official names for images in themes? Riccardo already stated that 
 things would work when using the name NSMenuArrow.
 
 If we don't use the nsmappings.strings for themes, themes may have to provide 
 a lot of duplicate files (e..g common_3DArrowRight.tiff, NSMenuArrow.tiff) 
 with the same contents. Not a huge problem… but it's ugly to have different 
 image lookup logic for images inside themes and other images.
 
 - The big doubt I am having with the change is that now GSTheme has to know 
 about that name mapping which was internal to NSImage up to now.
 A solution where similar code would be used inside the NSImage method 
 _setImagePath:name: seems a lot cleaner to me. If we build up that reverse 
 map you are using, all the necessary information should be available in that 
 method. I think this would belong into the else case of the if (image != 
 nil) test you introduced.
 
 I considered that - so +[NSImage _setImagePath:name:] for common_3DArrowRight 
 would also set the path for NSMenuArrow and anything else that maps to 
 common_3DArrowRight.
 
 There is a corner case with that; if the theme provides both NSMenuArrow and 
 common_3DArrowRight, the image used for NSMenuArrow depends on which call to 
 +[NSImage _setImagePath:name:] is made first.
 
 - Another way to get rid of the problem would be to completely remove the 
 name mapping from NSImage. I am a bit reluctant to propose this. That 
 mechanism has been around for a very long time and it allows us to have 
 clearer names. But with the theme code in place this mechanism isn't needed 
 that much any more.
 
 I committed a more radical redesign that is much cleaner, I think.
 Pasting my changelog comment:
 
I removed the step in theme activation where we call
+[NSImage _setImagePath:name:] on each image in the theme, and instead
modified +[NSImage _pathForImageNamed:] to also search the theme images
directory.
 
When a GSTheme activates now, it only calls +[NSImage _reloadCachedImages]
which checks all NSImage cached by name and reloads any whose path has
changed.
 
 My only worry is whether this will break the GTK or windows themes. IIRC they 
 replace the NSImage class and override +imageNamed:, so I think they'll still 
 work.
 
 Eric
 
 Cheers
 Fred
 
 On 06.04.2013 10:06, Eric Wasylishen wrote:
 Hi Riccardo,
 
 I committed a fix in r36474. What I ended up doing is, when a GSTheme 
 activates, it takes the image name - path dictionary of images in the 
 theme, and expands it by applying all of the nsmappings.strings mappings.
 
 So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll 
 produce a dictionary like:
 {
common_3DArrowRight : path/to/common_3DArrowRight.tiff,
NSMenuArrow : path/to/common_3DArrowRight.tiff,
 }
 
 This expanded set of images is then applied to the app state using 
 +[NSImage _setImagePath:name:], and the same expanded set is unregistered 
 when the theme deactivates.
 
 Hope this works for you, and the behaviour sounds sensible.
 
 Cheers.
 Eric
 
 On 2013-04-05, at 5:25 AM, Riccardo Mottola riccardo.mott...@libero.it 
 wrote:
 
 Hi all,
 
 On 03/28/13 16:40, Eric Wasylishen wrote:
 
 Hey Riccardo, check the nsmappings.strings file in Images. I think that 
 maps nsmenuarrow to one of the common_ images.
 
 back to the original problem, which is different from what German supposed.
 
 Let's remember that we have a mapping
 
 NSMenuArrow = common_3DArrowRight;
 
 
 Leaving out Thematic for a moment, I found that placing inside the Theme 
 Images a an image named
 
 common_3DArrowRight.tif
 
 doesn't work, while putting one called
 
 NSMenuArrow.tif
 
 works fine and the image gets loaded even dynamically when changing the 
 theme.
 
 
 ___
 Gnustep-dev mailing list
 Gnustep-dev@gnu.org
 https://lists.gnu.org/mailman/listinfo/gnustep-dev
 

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