Skip to content
This repository has been archived by the owner on Jul 10, 2018. It is now read-only.

Fix for alpha value out of range #18

Merged
merged 1 commit into from
Feb 24, 2017
Merged

Fix for alpha value out of range #18

merged 1 commit into from
Feb 24, 2017

Conversation

IvanRF
Copy link
Contributor

@IvanRF IvanRF commented Feb 24, 2017

No description provided.

@kirill-grouchnikov
Copy link
Owner

Do you have a specific scenario where alpha in this methods is outside the 0..1 range?

@IvanRF
Copy link
Contributor Author

IvanRF commented Feb 24, 2017

Not really. These pull requests come from 8 years of experience and testing with Substance+Flamingo 😄

I've seen this error and fixed it. This guy also made a lot of fixes including this one:
Icy-imaging@b1f6c8d
Icy-imaging@07a58b0

@kirill-grouchnikov
Copy link
Owner

The first fix doesn't seem to be right. It should be Math.min there.

As for doing it defensively, my strong preference is to do that only when it's actually shown to happen. Otherwise you're treating the symptom and not the core - which would be somewhere in Trident. And you can go really far with treating the symptoms, wrapping every method call to catch every kind of exception :)

@IvanRF
Copy link
Contributor Author

IvanRF commented Feb 24, 2017

The first fix doesn't seem to be right. It should be Math.min there.

I must say I was using Math.max(0.0f, Math.min(1.0f, alpha)); in my code

I saw you followed a similar approach on LafWidgetUtilities.getAlphaComposite(). I can't tell you the root cause, what I could tell you is that I didn't have any other error regarding the alpha.

@kirill-grouchnikov
Copy link
Owner

That code in LafWidgetUtilities is indeed there. It's been a while, so I can't really say why it's there.

Having said that, what do you mean by "any other error"? Does this mean that you are seeing alpha values outside the 0..1 range without this fix in SubstanceImageCreator? If not, then I won't be making this change.

@IvanRF
Copy link
Contributor Author

IvanRF commented Feb 24, 2017

What I meant is that without the fix I was having the "alpha value out of range" exception (sporadically), but after controlling the alpha range (by overriding your class in my project) I didn't have any other error regarding the alpha.

IvanRF referenced this pull request in Icy-imaging/Icy-Insubstantial Feb 24, 2017
@kirill-grouchnikov
Copy link
Owner

Is that under the latest 7.0?

@kirill-grouchnikov kirill-grouchnikov merged commit 65bff53 into kirill-grouchnikov:master Feb 24, 2017
@IvanRF
Copy link
Contributor Author

IvanRF commented Feb 24, 2017

Is that under the latest 7.0?

No, this comes from a previous version, I just found out about Substance back in the game today 👍

Here is another user with the same issue: Insubstantial#138
He posted the exception:

java.lang.IllegalArgumentException: alpha value out of range
    at java.awt.AlphaComposite.<init>(AlphaComposite.java:618)
    at java.awt.AlphaComposite.getInstance(AlphaComposite.java:683)
    at org.pushingpixels.substance.internal.utils.SubstanceImageCreator.getRadioButton(SubstanceImageCreator.java:880)
    at org.pushingpixels.substance.internal.ui.SubstanceRadioButtonUI.getIcon(SubstanceRadioButtonUI.java:206)
    at org.pushingpixels.substance.internal.ui.SubstanceRadioButtonUI.getDefaultIcon(SubstanceRadioButtonUI.java:311)
    at org.pushingpixels.substance.internal.ui.SubstanceRadioButtonUI.paint(SubstanceRadioButtonUI.java:346)
    at javax.swing.plaf.ComponentUI.update(ComponentUI.java:161)
    at org.pushingpixels.substance.internal.ui.SubstanceRadioButtonUI.__org__pushingpixels__substance__internal__ui__SubstanceRadioButtonUI__update(SubstanceRadioButtonUI.java)
    at org.pushingpixels.substance.internal.ui.SubstanceRadioButtonUI.update(SubstanceRadioButtonUI.java)
    at javax.swing.JComponent.paintComponent(JComponent.java:777)
    at javax.swing.JComponent.paint(JComponent.java:1053)
    at javax.swing.JComponent.paintToOffscreen(JComponent.java:5223)
    at javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1572)
    at javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1495)
    at javax.swing.RepaintManager.paint(RepaintManager.java:1265)
    at javax.swing.JComponent._paintImmediately(JComponent.java:5171)

@enwired
Copy link

enwired commented Feb 24, 2017

As I stated on Insubstantial bug 138, I saw this bug a few times, and I fixed it in my fork by adding a check to make sure the value was between 0.0 and 1.0. I never had a reliable way to trigger the bug, and I have never seen the problem again since my fix.

@enwired
Copy link

enwired commented Feb 24, 2017

I must say that I didn't really like putting in that fix without understanding why it was happening, but since I couldn't reproduce it reliably and don't understand the code sufficiently, I didn't see any other option.

@kirill-grouchnikov
Copy link
Owner

In theory even with floating arithmetic imprecisions you should never end up with a product of positive numbers being less than zero, and with a product of <= 1 numbers being more than one.

For this one I'm taking a more pragmatic stance and doing range clamp. This is the same as in the above-mentioned alpha composite method, as well as in the color interpolation code in SubstanceColorUtilities (used to be direct linear interpolation of each color channel, and is now using gamma space).

@enwired
Copy link

enwired commented Feb 24, 2017

"In theory ..." I wonder whether the bugs could be related to threading. Maybe non-atomic operations are happening with them on Swing and non-Swing threads.

But, whatever, I'm happy with the pragmatic "clamp" approach which has worked well for me for several years.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants