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