[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type BITMAPV2INFOHEADER & BITMAPV3INFOHEADE
From:       Jayathirth D V <jayathirth.d.v () oracle ! com>
Date:       2018-03-21 10:20:49
Message-ID: 966cb9f4-70aa-4ab8-87f7-17292c7b6000 () default
[Download RAW message or body]

Hi Prahalad,

Thank you for reviewing the change.

Yes we don't have any official Microsoft documentation today for BITMAPV2INFOHEADER & \
BITMAPV3INFOHEADER. We have only references to these undocumented DIB header types in \
http://fileformats.archiveteam.org/wiki/BMP , \
https://forums.adobe.com/message/3272950#3272950 & \
https://en.wikipedia.org/wiki/BMP_file_format 

I think that while BMP specification was getting enhanced Microsoft might have \
supported these formats but after that made BITMAPV4HEADER as officially supported \
format. Because of that we see that still some software like GIMP, Adobe Photoshop \
generate images with BITMAPV2INFOHEADER/ BITMAPV3INFOHEADER. Also all the official \
Microsoft documentation present as of now are from Windows SDK 2000 so we don't know \
what was the official journey of BMP specification pre 2000.

Regards,
Jay

-----Original Message-----
From: Prahalad Kumar Narayanan 
Sent: Wednesday, March 21, 2018 2:33 PM
To: Philip Race; Jayathirth D V; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type \
BITMAPV2INFOHEADER & BITMAPV3INFOHEADER is not supported in BMPImageReader

Hello Jay

Good day to you.

I looked into the code changes and they look good.

Appreciate the links that you have posted on JBS. 
I see that - the only reference material on Bitmap V2/V3 Info Header header exists on \
Adobe Forums (https://forums.adobe.com/message/3272950#3272950)

I could not find official reference from Microsoft describing the same.
Just curious to know : If there was any reason behind not documenting these versions \
of image header.

Thank you
Have a good day

Prahalad N.

----- Original Message -----
From: Phil Race
Sent: Tuesday, March 20, 2018 9:57 PM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type \
BITMAPV2INFOHEADER & BITMAPV3INFOHEADER is not supported in BMPImageReader

In that case the patch is fine.

-phil.
On 03/20/2018 12:15 AM, Jayathirth D V wrote:
HI Phil,
 
Please find my observation:
In case of DIB header type BITMAPINFOHEADER/ BITMAPV4HEADER/ BITMAPV5HEADER, \
Microsoft documentation(https://msdn.microsoft.com/en-us/library/dd183376(v=vs.85).aspx \
) mentions that mask values are valid only when compression type is BI_BITFIELDS. \
When compression type is BI_RGB which ii no compression, Microsoft document mentions \
that  
1) For 16 bpp : "The relative intensities of red, green, and blue are represented \
with five bits for each color component. The value for blue is in the least \
significant five bits, followed by five bits each for green and red. The most \
significant bit is not used.". So basically it should be RGB555. 2) For 32 bpp : \
"Each DWORD in the bitmap array represents the relative intensities of blue, green, \
and red for a pixel. The value for blue is in the least significant 8 bits, followed \
by 8 bits each for green and red. The high byte in each DWORD is not used". So \
basically it should be XRGB8888.  
This is why we have redMask = 0x7C00, greenMask = 0x3E0, blueMask = 0x1F in case of \
16bpp and redMask   = 0x00FF0000, greenMask = 0x0000FF00, blueMask  = 0x000000FFfor \
32bpp for all the three standard formats BITMAPINFOHEADER, BITMAPV4HEADER and \
BITMAPV5HEADER.  
Since BITMAPV2INFOHEADER & BITMAPV3INFOHEADER support falls in between that of other \
Microsoft documented DIB header types it is good that we follow the same approach in \
case of BITMAPV2INFOHEADER & BITMAPV3INFOHEADER also.  
Please let us know your inputs.
 
Thanks,
Jay
 
From: Phil Race
Sent: Monday, March 19, 2018 11:07 PM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type \
BITMAPV2INFOHEADER & BITMAPV3INFOHEADER is not supported in BMPImageReader  
Since the principal addition of these formats is to add explicit fields for \
supporting the bitmasks for accessing R/G/B/A it seems odd to see there is code like \
this which ignores it when the data is uncompressed by using these hardcoded values : \
456                             if ((int)compression == BI_RGB) {  457                \
redMask = 0x7C00;  458                                 greenMask = 0x3E0;
 459                                 blueMask = 0x1F;

I do see that it seems likely you copied this from the 108/124 case but I'd like to \
see some proof that this is correct.


-phil.

On 03/14/2018 03:39 AM, Jayathirth D V wrote:
Hello All,
 
Please review the following solution in JDK11 :
 
Bug : https://bugs.openjdk.java.net/browse/JDK-7031957
Webrev : http://cr.openjdk.java.net/~jdv/7031957/webrev.00/ 
 
Issue: If we try to read any BMP image of DIB header type BITMAPV2INFOHEADER/ \
BITMAPV3INFOHEADER, we get IOException mentioning the BMP image type in not yet \
implemented.  
Root cause:  BMPImageReader doesn't support DIB header types BITMAPV2INFOHEADER/ \
BITMAPV3INFOHEADER we support only BITMAPCOREHEADER, BITMAPINFOHEADER, BITMAPV4HEADER \
& BITMAPV5HEADER.  
Solution: Many other tools like GIMP, Microsoft PowerPoint, IrfanView support \
BITMAPV2INFOHEADER & BITMAPV3INFOHEADER format BMP images. We can consider \
BITMAPV2INFOHEADER & BITMAPV3INFOHEADER header types having functionality in between \
that of BITMAPINFOHEADER & BITMAPV4HEADER. BITMAPINFOHEADER with type BITFIELDS & \
extra 4 bytes for alpha channel or First 56 bytes of BITMAPV4HEADER is nothing but \
BITMAPV3INFOHEADER.  
To support BITMAPV2INFOHEADER & BITMAPV3INFOHEADER we can use similar approach of \
what we are doing while decoding first 56 bytes under BITMAPV4HEADER. So I have added \
additional "if()" to do the same, we can merge decoding of BITMAPV2INFOHEADER & \
BITMAPV3INFOHEADER at the same place where we are decoding BITMAPV4HEADER but we need \
to add many branch conditions to follow that approach.  
Thanks,
Jay
 
 


[prev in list] [next in list] [prev in thread] [next in thread]