[squeak-dev] Gradient Fills Failures

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Tue Mar 1 20:54:03 UTC 2011


Nice catch.

I think a simple solution would be to prevent mutation of colorRamp
inst. var, and replace mutation with a copy.
I don't see any use for these associations to be shared...
There are currently two mutators:
#firstColor:forMorph:hand: and #lastColor:forMorph:hand:

Another solution would be to put a colorRamp deepCopy in the factory
like you proposed
initPixelRampCache.
	^PixelRampCache := LRUCache size: 32 factory:[:key|
		(GradientFillStyle new colorRamp: key deepCopy) computePixelRampOfSize: 512]

I don't think that protecting the LRUCache is a good idea.
#deepCopy may cause infinite loop on object cyclic graphs
IMO the programmer shall be in charge of copying.

Anyway, the hash is inefficient indeed, most colorRamps share the same keys...
{
(((GradientFillStyle classPool at: #PixelRampCache) instVarAt:
(LRUCache allInstVarNames indexOf: 'values')) collect: [:e | e at: 2]
as: Set) size.
((GradientFillStyle classPool at: #PixelRampCache) instVarAt:
(LRUCache allInstVarNames indexOf: 'values')) size.
}
-> #(7 32)

I have no immediate solution for this one...

Nicolas

2011/3/1 glenpaling <glenpaling at rogers.com>:
> Modifications to Morphic gradients fail in current versions of Squeak (and
> Pharo). Using the halo menu to change the first or last color of a morph
> doesn't work. Despite changes to these colors, the initial gradient remains.
>
> I've traced the problem to false hits in GradientFillStyle's LRUCache.
> LRUCache acts similar to a dictionary. It uses a key and it's hash to store
> and retrieve values. GradientFillStyle uses uses its colorRamp--a mutable
> array of associations--as the key to the cache, for instance: {0.0->Color
> white. 1.0->Color red}. Subsequent modifications to colorRamp also changes
> the key IN the cache. This can result in false hits.
>
> This fault has always been present. Recent changes to the hash calculation
> in Associations increased its frequency. Association hash has been
> simplified to return the key. As a result all gradients created from the
> user interface have the same hash. Since the the lookup == the key and the
> hashes are the same, false hits are inevitable.
>
> To illustrate the problem execute the script below. The first two changes
> are done using the classes colorRamp modification function. Rather than
> modifying, the last change replaces colorRamp with a new one. Clearly the
> first item in the pixelRamp should change when the first color changes.
>
> gradient := GradientFillStyle new colorRamp: {0.0->Color red. 1.0->Color
> white}.
> Transcript
>        << 'first item for: ';
>        << gradient colorRamp printString; space;
>        << gradient pixelRamp first;
>        cr; flush.
>
> gradient firstColor: Color green forMorph: nil hand: nil.
> Transcript
>        << 'first item for: ';
>        << gradient colorRamp printString; space;
>        << gradient pixelRamp first;
>        cr; flush.
>
> gradient firstColor: Color yellow forMorph: nil hand: nil.
> Transcript
>        << 'first item for: ';
>        << gradient colorRamp printString; space;
>        << gradient pixelRamp first;
>        cr; flush.
>
> Transcript show: 'Now replace with a new colorRamp'; cr.
> newRamp := gradient colorRamp deepCopy.
> newRamp first value: Color black.
> gradient colorRamp: newRamp.
> Transcript
>        << 'first item for: ';
>        << gradient colorRamp printString; space;
>        << gradient pixelRamp first;
>        cr; flush.
>
>
> Results:
>
> first item for: {0.0->Color red . 1.0->Color white} 4294901760
> first item for: {0.0->Color green . 1.0->Color white} 4294901760
> first item for: {0.0->Color yellow . 1.0->Color white} 4294901760
> Now replace with a new colorRamp
> first item for: {0.0->Color black . 1.0->Color white} 4278190081
>
>
> What's the best way to fix this? Handing out your internals seems ill
> advised. I think GradientFillStyle should pass a deepCopy of colorRamp to
> LRUCache. Another solution is for LRUCache to make a deepCopy itself. This
> would provide protection for all users of LRUCache...provided that deepCopy
> will always be sufficient.
>
>
> Glen
>
> --
> View this message in context: http://forum.world.st/Gradient-Fills-Failures-tp3330016p3330016.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>
>



More information about the Squeak-dev mailing list