Issue 3770 in pharo: Gradient fills failure

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Issue 3770 in pharo: Gradient fills failure

pharo
Status: Accepted
Owner: [hidden email]

New issue 3770 by [hidden email]: Gradient fills failure
http://code.google.com/p/pharo/issues/detail?id=3770

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.