Re: NSImage / NSImageRep copy bug/problem

[Riccardo Mottola]

Hi Ivan,

thanks for lookin at the problem You found some valid cleaning up
Ivan Vučica wrote:
> - I assume PRImage's bitRep field ivar is represented by the property 
> bitmapRep; it is good practice to have the properties backed by either 
> ivars of the same name, or prefixed by underscore
Right. I refactored it to be cleaner. The code slowly morphed into this 
through incremental attempts, a cleanup was needed!

> Next, your implementation of -setBitmapRep: looks quite odd to me in 
> the first place. Do you really need to store a copy of the bitmap 
> representation in your subclass of NSImage? Does it make sense to 
> /just/ add a new representation (which may confuse NSImage)? Why not 
> implement -bitmapRep: as
>   return [[self representations] objectAtIndex:0];
> and -setBitmapRep: as something that, if [self bitmapRep] != rep, 
> clears away all representations and adds just one new one? That way 
> you avoid a lot of unnecessary juggling in your own code -- including 
> need to override copyWithZone:. Right now you have a very surprising 
> behavior -- calling -setBitmapRep: actually doesn't release the old 
> bitmap rep; it pretty much stays inside the representations array in 
> NSImage.
It is odd, byt I really want the behaviour your are describing. I used 
[[self representations] objectAtIndex:0] for years. Actually, no 
bitmapRep accessor was even needed. However, as I have discussed with 
Nikolaus in another email thread here on the list, I discovered that 
sometimes Cocoa instead of just adding new representation more suitable 
for the device drawing, removes the original representation. This was 
unexpected for me too, but is perhaps a good memory optimization. It is 
fine, but I have found no way to say NSImage, give me the original 
bitmap back! or a setting to keep it.
So the only thing I could think is to maintain a reference and add a 
retain to it, in case Cocoa wants do remove it from the representations 
array.

> 'if bitRep != rep' should be enough to ensure this doesn't happen. For 
> sake of code cleanliness, I'd still fix it as above.
I was checking for bitRep != rep ! Still, you are right, I cleaned up 
the code.
It is also unlikely to be the problem here, since the bitmapReps are 
either a new image or an explicit copy. But... tidying up is always good.

Thank you,

Riccardo