JDK-8126142 : SWTFXUtils#fromFXImage() broken
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 7u6
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2012-06-22
  • Updated: 2015-06-17
  • Resolved: 2012-06-25
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7
7u6Fixed
Related Reports
Relates :  
JDK-8116026 - PixelFormat.getByteBgraInstance.setArgb() appears to set the components in the wrong order
Description
SWTFXUtils#fromFXImage() no longer works

This is due to a change in PixelReader#getPixels() when the pixel format is getByteBgraInstance() (maybe others)

In the pixel byte array written by getPixels the color information used to be in this order:
offset + 0 = alpha = 0xFF000000
offset + 1 = red   = 0x00FF0000
offset + 2 = green = 0x0000FF00
offset + 3 = blue  = 0x000000FF

SWT Palette = 0xFF0000, 0xFF00, 0xFF = redMask, greenMask, blueMask

Now:

offset + 0 = blue  = 0xFF000000
offset + 1 = green = 0x00FF0000
offset + 2 = red   = 0x0000FF00
offset + 3 = alpha = 0x000000FF

SWT Palette = 0xFF00, 0xFF0000, 0xFF000000

I see that PixelReader#getPixels() and PixelWriter#setPixels() are consistent (the output of the first can be used as input for the second).
Thus, I suppose getPixel() had a bug in the past, which I "worked around" in the converter by using the wrong palette, and now that the bug is fixed the converter is broken.
Comments
verified in 2.2.0b17
16-07-2012
http://jfxsrc.us.oracle.com/javafx/2.2/scrum/graphics/rt-closed/rev/a4e0923f8b07 reviewed by Steve
25-06-2012
That's great. Felipe, please get this fix out ASAP and I will confirm that it fixed the problem. Thanks everyone.
22-06-2012
It looks like the Bgra.setArgb() method is wrong and we recently changed from using that method internally to using optimized loops (the optimized loops are correct), so thanks for noticing and pointing out that bug. Meanwhile, the question of why this code was impacted has been answered - the patch above is correct and responds to the new correct code we introduced when we optimized the buffer conversion methods and we will fix the old bug in setArgb() under RT-22885 in case anyone else is impacted.
22-06-2012
Thanks Jim, let me change my question: has the output of PixelReader#getPixels() changed since you first released the code ? (when pixel format == PixelFormat.getByteBgraInstance()) Disregard the eclipse code, for example: int width = (int) srcImg.getWidth(); int height = (int) srcImg.getHeight(); int bpr = width * 4; int dataSize = bpr * height; byte[] buffer = new byte[dataSize]; PixelReader pr = srcImg.getPixelReader(); WritablePixelFormat<ByteBuffer> pf = PixelFormat.getByteBgraInstance(); pr.getPixels(0, 0, width, height, pf, buffer, 0, bpr); int offset = 0; for (int y = 0; y<height; y++) { for (int x = 0; x<width; x++) { byte blue = buffer[offset++]; // in the first release first byte had alpha instead of blue (**) byte green = buffer[offset++]; // in the first release second byte had red instead of green (**) byte red = buffer[offset++]; // in the first release third byte had green instead of red (**) byte alpha = buffer[offset++]; // in the first release fourth byte had blue instead of alpha (**) } } Are the statements marked (**) true, ie. have you fixed a bug in getPixels() ?
22-06-2012
I have no idea how to verify the Eclipse code, but if you want to access the alpha data from a ByteBgra buffer then it is stored at pixeloffset+3 and that appears to be consistent with the inner loop part of the patch. Since the data is stored in a byte buffer and the palette seems to be constructed using integer masks I have no idea how to verify that part of the patch (other than, if it is doing what I think it is doing then it is assuming a big endian interpretation of the data in the byte buffer which seems odd and inefficient for use on Intel platforms).
22-06-2012
I verified that this patch fixes the problem on my mac: Felipes-MacBook-Pro-3:rt-closed felipe$ hg diff javafx-embed-swt/src/javafx/embed/swt/SWTFXUtils.java diff -r dd3f0763c4c1 javafx-embed-swt/src/javafx/embed/swt/SWTFXUtils.java --- a/javafx-embed-swt/src/javafx/embed/swt/SWTFXUtils.java Mon Jun 18 16:07:07 2012 -0700 +++ b/javafx-embed-swt/src/javafx/embed/swt/SWTFXUtils.java Fri Jun 22 10:15:34 2012 -0700 @@ -149,12 +149,12 @@ byte[] alphaData = new byte[width * height]; for (int y = 0, offset = 0, alphaOffset = 0; y < height; y++) { for (int x = 0; x < width; x++, offset += 4) { - byte alpha = buffer[offset]; - buffer[offset] = 0; + byte alpha = buffer[offset+3]; + buffer[offset+3] = 0; alphaData[alphaOffset++] = alpha; } } - PaletteData palette = new PaletteData(0xFF0000, 0xFF00, 0xFF); + PaletteData palette = new PaletteData(0xFF00, 0xFF0000, 0xFF000000); imageData = new ImageData(width, height, 32, palette, 4, buffer); imageData.alphaData = alphaData; return imageData;
22-06-2012
Jim: please read the description and let me know my analysis of the problem is correct. Thanks!
22-06-2012
The problem can be reproduced in any platforms using the toy: /rt-closed/toys/HelloFXCanvas/src/hellofxcanvas/HelloFXCanvasImages.java
22-06-2012