Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 341702

Summary:

[JFace] CompositeImageDescriptor mixes images with alpha channel wrong

Product:

[Eclipse Project] Platform

Reporter:

Stephan Wahlbrink <sw>

Component:

Assignee:

Oleg Besedin <ob1.eclipse>

Status:

VERIFIED FIXED

QA Contact:

Susan McCourt <susan>

Severity:

normal

Priority:

CC:

ob1.eclipse, remy.suen, Silenio_Quarti

Version:

3.7

Target Milestone:

3.7 M7

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Example for icon which is not painted correctly	none
Screenshot of the painted DemoIcon	none
Patch proposal	ob1.eclipse: iplog+
Updated patch	none

Description Stephan Wahlbrink

2011-04-02 14:26:52 EDT

Build Identifier: 3.7.0 I20110329-0800

The method used to mixes images with alpha channel in JFace's CompositeImageDescriptor seems to be wrong for some cases.

Reproducible: Always

Comment 1 Stephan Wahlbrink

2011-04-02 14:28:56 EDT

Created attachment 192415 [details]
Example for icon which is not painted correctly

Comment 2 Stephan Wahlbrink

2011-04-02 14:29:53 EDT

Created attachment 192416 [details]
Screenshot of the painted DemoIcon

Comment 3 Stephan Wahlbrink

2011-04-02 14:33:47 EDT

Created attachment 192417 [details]
Patch proposal

Comment 4 Oleg Besedin

2011-04-04 13:10:36 EDT

That code was originally added in bug 147072. 

Silenio, do you remember if that was a copy from SWT-something?

Comment 5 Silenio Quarti

2011-04-04 14:16:02 EDT

I believe it came from org.eclipse.swt.graphics.GC line#1367.

Comment 6 Oleg Besedin

2011-04-06 15:24:09 EDT

Created attachment 192665 [details]
Updated patch

Nice find and a very good fix! 

The current code is only correct if dstAlpha is 1.0. Which it is in many / most cases, but obviously not always. Even then the calculation of dstAlpha we have is wrong as for a semi-transparent overlay of opaque image it will produce result which will be partially transparent.

I'd like to make a few updates to the patch:

- Keep current formulas for when dstAlpha == 1.0, expect calculation of the result alpha channel which is alway going to be 1.0 in this case. This is both for performance and to minimize effect of the change.

- Add a check for re-calculated dstAlpha being 0 (it could happen if both original image and overlay are transparent at some point).

- Updated copyright

Comment 7 Oleg Besedin

2011-04-06 15:52:51 EDT

Updated patch applied to CVS Head. Thank you Stephan, excellent job!

Comment 8 Stephan Wahlbrink

2011-04-06 16:03:42 EDT

Thanks.

(In reply to comment #6)
> - Add a check for re-calculated dstAlpha being 0 (it could happen if both
> original image and overlay are transparent at some point).

I think it will not happen because sourceAlpha == 0 is already checked some lines above. Of course, the check is no problem.

Ciao,
Stephan

Comment 9 Oleg Besedin

2011-04-26 13:45:09 EDT

Verified in I20110425-1800.