Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1

[Laurent Bourgès]

Hi,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-091.1/

Changes in MarlinProperties only:
- getTileSize_Log2() & getTileWidth_Log2(); 32x32 tiles ie default = 5
(log2)

I hope it is good for now as tile settings are the same as in jdk9/10.

Regards,
Laurent

2018-03-21 21:44 GMT+01:00 Laurent Bourgès :

> Sergey,
>
> Le mer. 14 mars 2018 à 17:14, Sergey Bylokhov 
> a écrit :
>
>> On 13/03/2018 17:04, Sergey Bylokhov wrote:
>>
>>> I have started to look to this review, will run some closed tests and
>>> send a feedback soon.
>>>
>>
>> No issues found so far, +1.
>
>
> Thanks for your vote.
> I need another approval I suppose ?
>
> I will prepare another review asap reverting only tile size changes as
> using large tiles has performance drop on d3d & ogl that needs more work.
> It can be done later in follow-up issues.
>
> Phil do you agree the proposed plan ?
>
> Laurent
>

Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1

[Laurent Bourgès]

Sergey,

Le mer. 14 mars 2018 à 17:14, Sergey Bylokhov 
a écrit :

> On 13/03/2018 17:04, Sergey Bylokhov wrote:
>
>> I have started to look to this review, will run some closed tests and
>> send a feedback soon.
>>
>
> No issues found so far, +1.


Thanks for your vote.
I need another approval I suppose ?

I will prepare another review asap reverting only tile size changes as
using large tiles has performance drop on d3d & ogl that needs more work.
It can be done later in follow-up issues.

Phil do you agree the proposed plan ?

Laurent

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Thomas Stüfe]

Hi Phil!

thanks for pointing out the history, I was not aware of that.

I looked at that huffman coding and tried to determine whether the
underflow may happen in real life scenarios. I could at least not exclude
that possibility. I looked thru the mailing list threads - did someone
analyse and conclude for sure this was just a pointless compiler warning?

I would prefer the pragmatic solution (and IMHO also safer one) of fixing
this underflow in the proposed fashion. I had opened a bug report earlier
today. However, if someone already spent brain cycles on it and a patch -
in whatever form - is forthcoming, I do not want to butt in. In that case I
will close this bug again.

I would just like to see this fixed this because it affects us at SAP too.

Kind Regards, Thomas



On Wed, Mar 21, 2018 at 6:56 PM, Phil Race  wrote:

> I prefer the makefile fix, since we don't by policy, make changes to the
> imported libraries.
>
> On Jan 23rd [1] I expressed such a tool-chain specific makefile fix would
> be fine by me.
>
> Toolchain specific means ideally it would look like what Magnus wrote [2]
>
> Although you said GC 5.4.0 would need to be included in the logic.
>
> If it can be shown to affect current / future versions of gcc then it
> could be unqualified.
>
> I think we've just been waiting for a webrev since then ..
>
> -phil.
>
> [1] http://mail.openjdk.java.net/pipermail/2d-dev/2018-January/008855.html
> [2] http://mail.openjdk.java.net/pipermail/build-dev/2018-
> January/020695.html
>
>
> On 03/21/2018 09:53 AM, Adam Farley8 wrote:
>
> :)
>
> > Hi Adam,
> >
> > no problem. I'll open a bug and if necessary find a second reviewer.
> Thanks for fixing, maybe I can stop building with warnings disabled on our
> s390 machines now.
> >
> > ..Thomas
> >
> > > On Wed, Mar 21, 2018 at 5:10 PM, Andrew Leonard
>   wrote:
> > > Hi Thomas,
> > > I'm a "contributor", but not a "committer", so not on that list,
> didn't even know that
> > > list existed! I was sort of assuming since it was a trivial change,
> and the request was
> > > for a review, i'd chip in...!
> > > Thanks
> > > Andrew
> >
> > > Andrew Leonard
> > > Java Runtimes Development
> > > IBM Hursley
> > > IBM United Kingdom Ltd
> > > Phone internal: 245913, external: 01962 815913
> > > internet email: andrew_m_leon...@uk.ibm.com
> >
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
>

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Alex Menkov]

Hi Magnus,

> I have tested the following:
>   * On my linux machine, failure to load libjsound.so was not fatal.

In Platform.java:
  54 loadLibraries();
  55 readProperties();

and readProperties calls native nIsBigEndian

if libjsound loading fails I'd expect nIsBigEndian fails too.

--alex

On 03/21/2018 07:09, Magnus Ihse Bursie wrote:

On 2018-03-16 17:49, Alex Menkov wrote:



On 03/15/2018 13:09, Magnus Ihse Bursie wrote:



15 mars 2018 kl. 20:13 skrev Phil Race :


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff


Yes, I think it is something like that for Linux
ie if at runtime a dependent-but-not-essential .so was not
installed it was not fatal. I don't know to what extent this is no 
longer a

possible issue, or one that matters.


I have not heard of any mainstream Linux distro in years that was 
lacking ALSA.


If ALSA was not present, will the libraries fall back to OSS, or will 
there be just no sound available?


No sound.
OSS support was dropped many years ago (IIRC in jdk7)

In any case, I think that whatever Linux distros we're targeting as 
supported, ALSA will be present.


Alex, did I understand you correctly that in any case, a separate 
Windows library is always unnecessary, since we can rely on 
DirectAudio always being present in our supported versions of Windows?


Yes, that's right.
Windows always has DirectSound pre-installed and its version is 
greater than required (IIRC javasoundds requires DirectX 5).


For now failure of libjsound loading is fatal (see 
com.sun.media.sound.Platform.loadLibraries()), loading of extra libs 
is non-fatal.
I believe libjsound loading failure should be made non-fatal, then all 
the functionality will remain the same as we have now.


Ok.

Here is an updated webrev. I have made the following changes:
* libjavasoundalsa and libjavasoundds is now folded into the main 
libjavasound native library, so there's exactly one library built on all 
platforms.

* Loading of libjsound is made non-fatal.
* I have cleaned out all obvious parts of the code that handle multiple 
libraries. Since loading the native library is now a all-or-nothing 
situation, the checks for various subsystems have been turned into a 
generic check if the native library is loaded.


There is a lot of defines like USE_DSOUND which are always true. This 
could probably be cleaned up further, but it is not a build issue so I'm 
leaving that to the client team to handle.


I have tested the following:
  * COMPARE_BUILD shows me just the expected changes in the build.
  * On my linux machine, failure to load libjsound.so was not fatal.
  * I have looked for sound tests. I found the test/jdk/javax/sound 
suite, which was included in tier3. So I've run tier3 testing on all 
platforms using our internal test system, and all tests pass.


I don't know if there is any other tests I should run. If so, let me know.

Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.03


/Magnus



--alex



/Magnus



-phil.


On 03/15/2018 12:06 PM, Alex Menkov wrote:



On 03/15/2018 11:44, Magnus Ihse Bursie wrote:

On 2018-03-15 18:23, Phil Race wrote:
I wondered if that might be the case since it was a "BSD" port .. 
using X11 ..


Maybe we should be getting rid of them ?
I agree, we should delete them. I just shuffled them around in the 
hope that they would be useful for a potential future bsd port, 
but if/when that happens, we can dig them out from mercurial.


A short explanation of how the files moved. The sound library is 
apparently composed of either a single library (solaris and 
macosx) or two libraries (linux and windows). Two building blocks, 
MIDI + ports and DirectAudio is used for all platforms, but they 
go into either the main library (libjsound) or the helper library.


For Windows, MIDI+Ports go into libjsound, and DirectAudio go into 
libjsoundds. On Linux, MIDI+Ports and DirectAudio go into 
libjsoundalsa. On Macosx and Solaris, MIDI+Ports and DirectAudio 
go into the main libjsound.


I have no idea why this split is necessary, but this is how the 
libraries de facto is compiled, and the code needs to match that. 
If it would be possible to move libjsoundds and libjsoundalsa into 
libjsound directly, things would be greatly simplified.


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff. If it's not available (or old unsupported 
version is installed), libjsound stuff continues to work (in 
pre-OpenJDK libjsound supported WaveIn/WaveOut on Windows and OSS 
on Linux).
For now Windows (DirectSound) libjsoundds stuff can be merged into 
libjsound, but I'm not sure we can rely on ALSA is always available 
on Linux (but most likely if ALSA is not available, libjsound does 
not provide any functionality, so I suppose libjsoundalsa stuff can 
be moved to libjsound as well)


--alex



/Magnus



-phil.


On 03/15/2018 10:21 AM, Erik Joelsson 

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Phil Race]

I prefer the makefile fix, since we don't by policy, make changes to the 
imported libraries.


On Jan 23rd [1] I expressed such a tool-chain specific makefile fix 
would be fine by me.


Toolchain specific means ideally it would look like what Magnus wrote [2]

Although you said GC 5.4.0 would need to be included in the logic.

If it can be shown to affect current / future versions of gcc then it 
could be unqualified.


I think we've just been waiting for a webrev since then ..

-phil.

[1] http://mail.openjdk.java.net/pipermail/2d-dev/2018-January/008855.html
[2] 
http://mail.openjdk.java.net/pipermail/build-dev/2018-January/020695.html


On 03/21/2018 09:53 AM, Adam Farley8 wrote:

:)

> Hi Adam,
> 
> no problem. I'll open a bug and if necessary find a second reviewer. 
Thanks for fixing, maybe I can stop building with warnings disabled on 
our s390 machines now.
> 
> ..Thomas
> 
> > On Wed, Mar 21, 2018 at 5:10 PM, Andrew Leonard 
 wrote:

> > Hi Thomas,
> > I'm a "contributor", but not a "committer", so not on that list, didn't 
even know that
> > list existed! I was sort of assuming since it was a trivial change, 
and the request was

> > for a review, i'd chip in...!
> > Thanks
> > Andrew
> 
> > Andrew Leonard

> > Java Runtimes Development
> > IBM Hursley
> > IBM United Kingdom Ltd
> > Phone internal: 245913, external: 01962 815913
> > internet email: andrew_m_leon...@uk.ibm.com
> 


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.

Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

[OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Thomas Stüfe]

Hi all,

may I please have another review for this really trivial change. It fixes a
gcc warning on s390 and ppc. Also, it is probably a good idea to fix this.

bug: https://bugs.openjdk.java.net/browse/JDK-8200052
webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/

This was contributed by Adam Farley at IBM.

I already reviewed this. I also test-built on zlinux and it works.

Thanks, Thomas

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Adam Farley8]

:)

> Hi Adam,
> 
> no problem. I'll open a bug and if necessary find a second reviewer. 
Thanks for fixing, maybe I can stop building with warnings disabled on our 
s390 machines now.
> 
> ..Thomas
> 
> > On Wed, Mar 21, 2018 at 5:10 PM, Andrew Leonard 
 wrote:
> > Hi Thomas, 
> > I'm a "contributor", but not a "committer", so not on that list, 
didn't even know that 
> > list existed! I was sort of assuming since it was a trivial change, 
and the request was 
> > for a review, i'd chip in...! 
> > Thanks 
> > Andrew 
> 
> > Andrew Leonard
> > Java Runtimes Development
> > IBM Hursley
> > IBM United Kingdom Ltd
> > Phone internal: 245913, external: 01962 815913
> > internet email: andrew_m_leon...@uk.ibm.com 
> 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Thomas Stüfe]

Hi Adam,

no problem. I'll open a bug and if necessary find a second reviewer. Thanks
for fixing, maybe I can stop building with warnings disabled on our s390
machines now.

..Thomas

On Wed, Mar 21, 2018 at 5:10 PM, Andrew Leonard  wrote:

> Hi Thomas,
> I'm a "contributor", but not a "committer", so not on that list, didn't
> even know that list existed! I was sort of assuming since it was a trivial
> change, and the request was for a review, i'd chip in...!
> Thanks
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leon...@uk.ibm.com
>
>
>
>
> From:"Thomas Stüfe" 
> To:Adam Farley8 , Andrew Leonard <
> andrew_m_leon...@uk.ibm.com>
> Cc:2d-dev <2d-dev@openjdk.java.net>
> Date:21/03/2018 15:42
> Subject:Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to
> compile jchuff.c
> --
>
>
>
> Hi,
>
> @Andrew Leonard: Sorry, I cannot find your name in
> *http://openjdk.java.net/census#jdk*
> .
> Are you a jdk contributor/reviewer, and if yes, what is your user name?
>
> @openjdk 2d folks: I also need to know: does the 2d project have any
> special rules about who can push or can I just push (I am jdk reviewer,
> census name stuefe)?
>
> Also, if A.Leonard is not a valid reviewer, could I still push this under
> the "trivial" rule? The change is quite trivial :)
>
>
>
> Best,
>
> Thomas
>
>
>
> On Wed, Mar 21, 2018 at 4:17 PM, Adam Farley8 <*adam.far...@uk.ibm.com*
> > wrote:
> cc'ing Tom directly in case this fell into a digest.
>
> > I've reviewed it. The change looks good, it's a good programming
> practice while loop now :-)
> > Cheers
> > Andrew
> >
> > Andrew Leonard
> > Java Runtimes Development
> > IBM Hursley
> > IBM United Kingdom Ltd
> > Phone internal: 245913, external: 01962 815913
> > internet email: *andrew_m_leon...@uk.ibm.com*
> 
> >
> > > Hi Tom,
> > >
> > > Much obliged. :)
> > >
> > > Any volunteers to be the 2nd reviewer?
> > >
> > > Best Regards
> > >
> > > Adam Farley
> > >
> > > > Hi Adam,
> > > >
> > > > this patch looks good. I can sponsor this for you if noone else
> steps up, but we need a second reviewer, preferably one from the 2d project.
> > > >
> > > > Best Regards, Thomas
> > > >
> > > > On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8 <
> *adam.far...@uk.ibm.com* > wrote:
> > > > Hi All,
> > > >
> > > > If committers really don't want this code, we could always try
> fixing the code that the warning
> > > > is complaining about.
> > > >
> > > > -- Change 
> > > > ---
>
> > > > diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > @@ -805,7 +805,7 @@
> > > >for (i = MAX_CLEN; i > 16; i--) {
> > > >  while (bits[i] > 0) {
> > > >j = i - 2;/* find length of new prefix to be
> used */
> > > > -  while (bits[j] == 0)
> > > > +  while ((bits[j] == 0) && (j > 0))
> > > >  j--;
> > > >
> > > >bits[i] -= 2; /* remove two symbols */
> > > > -- End of Change
> ---
> > > >
> > > > Again, it's a small, simple change that fixes a build break on two
> platforms.
> > > >
> > > > Either fix will solve this problem.
> > > >
> > > > Best Regards
> > > >
> > > > Adam Farley
> > > >
> > > > > Hi All,
> > > > >
> > > > > I ask for a committer to add one word to
> make/lib/Awt2dLibraries.gmk to solve a build break.
> > > > >
> > > > > We need to go to line 495 and add array-bounds into the list of
> disabled warnings.
> > > > >
> > > > > So this:
> > > > >
> > > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value, \
> > > > >
> > > > > becomes this:
> > > > >
> > > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value array-bounds, \
> > > > >
> > > > > This fixes a build-breaking problem which occurs if you don't
> disable
> > > > > errors-as-warnings on zLinux or Linux for ppcle.
> > > > >
> > > > > Best Regards
> > > > >
> > > > > Adam Farley
> > > > >
> > > > > P.S. For further background, see this:
> > > > >
> *http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html*
> 

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Andrew Leonard]

Hi Thomas,
I'm a "contributor", but not a "committer", so not on that list, didn't 
even know that list existed! I was sort of assuming since it was a trivial 
change, and the request was for a review, i'd chip in...!
Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 




From:   "Thomas Stüfe" 
To: Adam Farley8 , Andrew Leonard 

Cc: 2d-dev <2d-dev@openjdk.java.net>
Date:   21/03/2018 15:42
Subject:Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to 
compile jchuff.c



Hi,

@Andrew Leonard: Sorry, I cannot find your name in 
http://openjdk.java.net/census#jdk. Are you a jdk contributor/reviewer, 
and if yes, what is your user name?

@openjdk 2d folks: I also need to know: does the 2d project have any 
special rules about who can push or can I just push (I am jdk reviewer, 
census name stuefe)? 

Also, if A.Leonard is not a valid reviewer, could I still push this under 
the "trivial" rule? The change is quite trivial :)



Best,

Thomas



On Wed, Mar 21, 2018 at 4:17 PM, Adam Farley8  
wrote:
cc'ing Tom directly in case this fell into a digest. 

> I've reviewed it. The change looks good, it's a good programming 
practice while loop now :-) 
> Cheers 
> Andrew 
> 
> Andrew Leonard 
> Java Runtimes Development 
> IBM Hursley 
> IBM United Kingdom Ltd 
> Phone internal: 245913, external: 01962 815913 
> internet email: andrew_m_leon...@uk.ibm.com 
> 
> > Hi Tom, 
> > 
> > Much obliged. :) 
> > 
> > Any volunteers to be the 2nd reviewer? 
> > 
> > Best Regards 
> > 
> > Adam Farley 
> > 
> > > Hi Adam, 
> > > 
> > > this patch looks good. I can sponsor this for you if noone else 
steps up, but we need a second reviewer, preferably one from the 2d 
project. 
> > > 
> > > Best Regards, Thomas 
> > > 
> > > On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8 <
adam.far...@uk.ibm.com> wrote: 
> > > Hi All, 
> > > 
> > > If committers really don't want this code, we could always try 
fixing the code that the warning 
> > > is complaining about. 
> > > 
> > > -- Change 
--- 
> > > diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > @@ -805,7 +805,7 @@ 
> > >for (i = MAX_CLEN; i > 16; i--) { 
> > >  while (bits[i] > 0) { 
> > >j = i - 2;/* find length of new prefix to be 
used */ 
> > > -  while (bits[j] == 0) 
> > > +  while ((bits[j] == 0) && (j > 0)) 
> > >  j--; 
> > > 
> > >bits[i] -= 2; /* remove two symbols */ 
> > > -- End of Change 
--- 
> > > 
> > > Again, it's a small, simple change that fixes a build break on two 
platforms. 
> > > 
> > > Either fix will solve this problem. 
> > > 
> > > Best Regards 
> > > 
> > > Adam Farley 
> > > 
> > > > Hi All, 
> > > > 
> > > > I ask for a committer to add one word to 
make/lib/Awt2dLibraries.gmk to solve a build break. 
> > > > 
> > > > We need to go to line 495 and add array-bounds into the list of 
disabled warnings. 
> > > > 
> > > > So this: 
> > > > 
> > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value, \ 
> > > > 
> > > > becomes this: 
> > > > 
> > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value array-bounds, \ 
> > > > 
> > > > This fixes a build-breaking problem which occurs if you don't 
disable 
> > > > errors-as-warnings on zLinux or Linux for ppcle. 
> > > > 
> > > > Best Regards 
> > > > 
> > > > Adam Farley 
> > > > 
> > > > P.S. For further background, see this: 
> > > > 
http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html 
> > > 
> > > 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU




Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Thomas Stüfe]

Hi,

@Andrew Leonard: Sorry, I cannot find your name in
http://openjdk.java.net/census#jdk. Are you a jdk contributor/reviewer, and
if yes, what is your user name?

@openjdk 2d folks: I also need to know: does the 2d project have any
special rules about who can push or can I just push (I am jdk reviewer,
census name stuefe)?

Also, if A.Leonard is not a valid reviewer, could I still push this under
the "trivial" rule? The change is quite trivial :)



Best,

Thomas



On Wed, Mar 21, 2018 at 4:17 PM, Adam Farley8 
wrote:

> cc'ing Tom directly in case this fell into a digest.
>
> > I've reviewed it. The change looks good, it's a good programming
> practice while loop now :-)
> > Cheers
> > Andrew
> >
> > Andrew Leonard
> > Java Runtimes Development
> > IBM Hursley
> > IBM United Kingdom Ltd
> > Phone internal: 245913, external: 01962 815913
> > internet email: andrew_m_leon...@uk.ibm.com
> >
> > > Hi Tom,
> > >
> > > Much obliged. :)
> > >
> > > Any volunteers to be the 2nd reviewer?
> > >
> > > Best Regards
> > >
> > > Adam Farley
> > >
> > > > Hi Adam,
> > > >
> > > > this patch looks good. I can sponsor this for you if noone else
> steps up, but we need a second reviewer, preferably one from the 2d project.
> > > >
> > > > Best Regards, Thomas
> > > >
> > > > On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8 <
> adam.far...@uk.ibm.com> wrote:
> > > > Hi All,
> > > >
> > > > If committers really don't want this code, we could always try
> fixing the code that the warning
> > > > is complaining about.
> > > >
> > > > -- Change 
> > > > ---
>
> > > > diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> > > > @@ -805,7 +805,7 @@
> > > >for (i = MAX_CLEN; i > 16; i--) {
> > > >  while (bits[i] > 0) {
> > > >j = i - 2;/* find length of new prefix to be
> used */
> > > > -  while (bits[j] == 0)
> > > > +  while ((bits[j] == 0) && (j > 0))
> > > >  j--;
> > > >
> > > >bits[i] -= 2; /* remove two symbols */
> > > > -- End of Change
> ---
> > > >
> > > > Again, it's a small, simple change that fixes a build break on two
> platforms.
> > > >
> > > > Either fix will solve this problem.
> > > >
> > > > Best Regards
> > > >
> > > > Adam Farley
> > > >
> > > > > Hi All,
> > > > >
> > > > > I ask for a committer to add one word to
> make/lib/Awt2dLibraries.gmk to solve a build break.
> > > > >
> > > > > We need to go to line 495 and add array-bounds into the list of
> disabled warnings.
> > > > >
> > > > > So this:
> > > > >
> > > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value, \
> > > > >
> > > > > becomes this:
> > > > >
> > > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value array-bounds, \
> > > > >
> > > > > This fixes a build-breaking problem which occurs if you don't
> disable
> > > > > errors-as-warnings on zLinux or Linux for ppcle.
> > > > >
> > > > > Best Regards
> > > > >
> > > > > Adam Farley
> > > > >
> > > > > P.S. For further background, see this:
> > > > > http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/
> 008958.html
> > > >
> > > >
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-03-21 07:09, Magnus Ihse Bursie wrote:

On 2018-03-16 17:49, Alex Menkov wrote:



On 03/15/2018 13:09, Magnus Ihse Bursie wrote:



15 mars 2018 kl. 20:13 skrev Phil Race :


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff


Yes, I think it is something like that for Linux
ie if at runtime a dependent-but-not-essential .so was not
installed it was not fatal. I don't know to what extent this is no 
longer a

possible issue, or one that matters.


I have not heard of any mainstream Linux distro in years that was 
lacking ALSA.


If ALSA was not present, will the libraries fall back to OSS, or 
will there be just no sound available?


No sound.
OSS support was dropped many years ago (IIRC in jdk7)

In any case, I think that whatever Linux distros we're targeting as 
supported, ALSA will be present.


Alex, did I understand you correctly that in any case, a separate 
Windows library is always unnecessary, since we can rely on 
DirectAudio always being present in our supported versions of Windows?


Yes, that's right.
Windows always has DirectSound pre-installed and its version is 
greater than required (IIRC javasoundds requires DirectX 5).


For now failure of libjsound loading is fatal (see 
com.sun.media.sound.Platform.loadLibraries()), loading of extra libs 
is non-fatal.
I believe libjsound loading failure should be made non-fatal, then 
all the functionality will remain the same as we have now.


Ok.

Here is an updated webrev. I have made the following changes:
* libjavasoundalsa and libjavasoundds is now folded into the main 
libjavasound native library, so there's exactly one library built on 
all platforms.

* Loading of libjsound is made non-fatal.
* I have cleaned out all obvious parts of the code that handle 
multiple libraries. Since loading the native library is now a 
all-or-nothing situation, the checks for various subsystems have been 
turned into a generic check if the native library is loaded.


There is a lot of defines like USE_DSOUND which are always true. This 
could probably be cleaned up further, but it is not a build issue so 
I'm leaving that to the client team to handle.


I have tested the following:
 * COMPARE_BUILD shows me just the expected changes in the build.
 * On my linux machine, failure to load libjsound.so was not fatal.
 * I have looked for sound tests. I found the test/jdk/javax/sound 
suite, which was included in tier3. So I've run tier3 testing on all 
platforms using our internal test system, and all tests pass.


I don't know if there is any other tests I should run. If so, let me 
know.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.03


/Magnus



--alex



/Magnus



-phil.


On 03/15/2018 12:06 PM, Alex Menkov wrote:



On 03/15/2018 11:44, Magnus Ihse Bursie wrote:

On 2018-03-15 18:23, Phil Race wrote:
I wondered if that might be the case since it was a "BSD" port 
.. using X11 ..


Maybe we should be getting rid of them ?
I agree, we should delete them. I just shuffled them around in 
the hope that they would be useful for a potential future bsd 
port, but if/when that happens, we can dig them out from mercurial.


A short explanation of how the files moved. The sound library is 
apparently composed of either a single library (solaris and 
macosx) or two libraries (linux and windows). Two building 
blocks, MIDI + ports and DirectAudio is used for all platforms, 
but they go into either the main library (libjsound) or the 
helper library.


For Windows, MIDI+Ports go into libjsound, and DirectAudio go 
into libjsoundds. On Linux, MIDI+Ports and DirectAudio go into 
libjsoundalsa. On Macosx and Solaris, MIDI+Ports and DirectAudio 
go into the main libjsound.


I have no idea why this split is necessary, but this is how the 
libraries de facto is compiled, and the code needs to match that. 
If it would be possible to move libjsoundds and libjsoundalsa 
into libjsound directly, things would be greatly simplified.


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff. If it's not available (or old unsupported 
version is installed), libjsound stuff continues to work (in 
pre-OpenJDK libjsound supported WaveIn/WaveOut on Windows and OSS 
on Linux).
For now Windows (DirectSound) libjsoundds stuff can be merged into 
libjsound, but I'm not sure we can rely on ALSA is always 
available on Linux (but most likely if ALSA is not available, 
libjsound does not provide any functionality, so I suppose 
libjsoundalsa stuff can be moved to libjsound as well)


--alex



/Magnus



-phil.


On 03/15/2018 10:21 AM, Erik Joelsson wrote:
Digging a bit, those files came with the initial Macosx 
support. It doesn't look like they were ever used.


/Erik



On 2018-03-15 09:53, Phil Race wrote:
It is very hard to follow all the moved around files, but one 
thing

that sticks out is there is a "bsd" directory 

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Adam Farley8]

cc'ing Tom directly in case this fell into a digest.

> I've reviewed it. The change looks good, it's a good programming 
practice while loop now :-)
> Cheers
> Andrew
> 
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leon...@uk.ibm.com 
> 
> > Hi Tom,
> > 
> > Much obliged. :)
> > 
> > Any volunteers to be the 2nd reviewer?
> > 
> > Best Regards
> > 
> > Adam Farley
> > 
> > > Hi Adam,
> > > 
> > > this patch looks good. I can sponsor this for you if noone else 
steps up, but we need a second reviewer, preferably one from the 2d 
project.
> > > 
> > > Best Regards, Thomas
> > > 
> > > On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8 
 wrote:
> > > Hi All, 
> > > 
> > > If committers really don't want this code, we could always try 
fixing the code that the warning 
> > > is complaining about. 
> > > 
> > > -- Change 
--- 
> > > diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> > > @@ -805,7 +805,7 @@ 
> > >for (i = MAX_CLEN; i > 16; i--) { 
> > >  while (bits[i] > 0) { 
> > >j = i - 2;/* find length of new prefix to be 
used */ 
> > > -  while (bits[j] == 0) 
> > > +  while ((bits[j] == 0) && (j > 0)) 
> > >  j--; 
> > > 
> > >bits[i] -= 2; /* remove two symbols */
> > > -- End of Change 
--- 
> > > 
> > > Again, it's a small, simple change that fixes a build break on two 
platforms. 
> > > 
> > > Either fix will solve this problem. 
> > > 
> > > Best Regards
> > > 
> > > Adam Farley 
> > > 
> > > > Hi All, 
> > > > 
> > > > I ask for a committer to add one word to 
make/lib/Awt2dLibraries.gmk to solve a build break. 
> > > > 
> > > > We need to go to line 495 and add array-bounds into the list of 
disabled warnings. 
> > > > 
> > > > So this: 
> > > > 
> > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value, \ 
> > > > 
> > > > becomes this: 
> > > > 
> > > > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value array-bounds, \ 
> > > > 
> > > > This fixes a build-breaking problem which occurs if you don't 
disable 
> > > > errors-as-warnings on zLinux or Linux for ppcle. 
> > > > 
> > > > Best Regards 
> > > > 
> > > > Adam Farley 
> > > > 
> > > > P.S. For further background, see this: 
> > > > 
http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html 
> > > 
> > > 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

[OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Andrew Leonard]

I've reviewed it. The change looks good, it's a good programming practice 
while loop now :-)
Cheers
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Magnus Ihse Bursie]

On 2018-03-16 17:49, Alex Menkov wrote:



On 03/15/2018 13:09, Magnus Ihse Bursie wrote:



15 mars 2018 kl. 20:13 skrev Phil Race :


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff


Yes, I think it is something like that for Linux
ie if at runtime a dependent-but-not-essential .so was not
installed it was not fatal. I don't know to what extent this is no 
longer a

possible issue, or one that matters.


I have not heard of any mainstream Linux distro in years that was 
lacking ALSA.


If ALSA was not present, will the libraries fall back to OSS, or will 
there be just no sound available?


No sound.
OSS support was dropped many years ago (IIRC in jdk7)

In any case, I think that whatever Linux distros we're targeting as 
supported, ALSA will be present.


Alex, did I understand you correctly that in any case, a separate 
Windows library is always unnecessary, since we can rely on 
DirectAudio always being present in our supported versions of Windows?


Yes, that's right.
Windows always has DirectSound pre-installed and its version is 
greater than required (IIRC javasoundds requires DirectX 5).


For now failure of libjsound loading is fatal (see 
com.sun.media.sound.Platform.loadLibraries()), loading of extra libs 
is non-fatal.
I believe libjsound loading failure should be made non-fatal, then all 
the functionality will remain the same as we have now.


Ok.

Here is an updated webrev. I have made the following changes:
* libjavasoundalsa and libjavasoundds is now folded into the main 
libjavasound native library, so there's exactly one library built on all 
platforms.

* Loading of libjsound is made non-fatal.
* I have cleaned out all obvious parts of the code that handle multiple 
libraries. Since loading the native library is now a all-or-nothing 
situation, the checks for various subsystems have been turned into a 
generic check if the native library is loaded.


There is a lot of defines like USE_DSOUND which are always true. This 
could probably be cleaned up further, but it is not a build issue so I'm 
leaving that to the client team to handle.


I have tested the following:
 * COMPARE_BUILD shows me just the expected changes in the build.
 * On my linux machine, failure to load libjsound.so was not fatal.
 * I have looked for sound tests. I found the test/jdk/javax/sound 
suite, which was included in tier3. So I've run tier3 testing on all 
platforms using our internal test system, and all tests pass.


I don't know if there is any other tests I should run. If so, let me know.

Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.03


/Magnus



--alex



/Magnus



-phil.


On 03/15/2018 12:06 PM, Alex Menkov wrote:



On 03/15/2018 11:44, Magnus Ihse Bursie wrote:

On 2018-03-15 18:23, Phil Race wrote:
I wondered if that might be the case since it was a "BSD" port .. 
using X11 ..


Maybe we should be getting rid of them ?
I agree, we should delete them. I just shuffled them around in the 
hope that they would be useful for a potential future bsd port, 
but if/when that happens, we can dig them out from mercurial.


A short explanation of how the files moved. The sound library is 
apparently composed of either a single library (solaris and 
macosx) or two libraries (linux and windows). Two building blocks, 
MIDI + ports and DirectAudio is used for all platforms, but they 
go into either the main library (libjsound) or the helper library.


For Windows, MIDI+Ports go into libjsound, and DirectAudio go into 
libjsoundds. On Linux, MIDI+Ports and DirectAudio go into 
libjsoundalsa. On Macosx and Solaris, MIDI+Ports and DirectAudio 
go into the main libjsound.


I have no idea why this split is necessary, but this is how the 
libraries de facto is compiled, and the code needs to match that. 
If it would be possible to move libjsoundds and libjsoundalsa into 
libjsound directly, things would be greatly simplified.


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff. If it's not available (or old unsupported 
version is installed), libjsound stuff continues to work (in 
pre-OpenJDK libjsound supported WaveIn/WaveOut on Windows and OSS 
on Linux).
For now Windows (DirectSound) libjsoundds stuff can be merged into 
libjsound, but I'm not sure we can rely on ALSA is always available 
on Linux (but most likely if ALSA is not available, libjsound does 
not provide any functionality, so I suppose libjsoundalsa stuff can 
be moved to libjsound as well)


--alex



/Magnus



-phil.


On 03/15/2018 10:21 AM, Erik Joelsson wrote:
Digging a bit, those files came with the initial Macosx support. 
It doesn't look like they were ever used.


/Erik



On 2018-03-15 09:53, Phil Race wrote:
It is very hard to follow all the moved around files, but one 
thing

that sticks out is there is a "bsd" directory created and I can't
work out how the files in there are used.
If they are for a BSD port 

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Adam Farley8]

Hi Tom,

Much obliged. :)

Any volunteers to be the 2nd reviewer?

Best Regards

Adam Farley

> Hi Adam,
> 
> this patch looks good. I can sponsor this for you if noone else steps 
up, but we need a second reviewer, preferably one from the 2d project.
> 
> Best Regards, Thomas
> 
> On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8  
wrote:
> Hi All, 
> 
> If committers really don't want this code, we could always try fixing 
the code that the warning 
> is complaining about. 
> 
> -- Change 
--- 
> diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c 
> @@ -805,7 +805,7 @@ 
>for (i = MAX_CLEN; i > 16; i--) { 
>  while (bits[i] > 0) { 
>j = i - 2;/* find length of new prefix to be used 
*/ 
> -  while (bits[j] == 0) 
> +  while ((bits[j] == 0) && (j > 0)) 
>  j--; 
> 
>bits[i] -= 2; /* remove two symbols */
> -- End of Change 
--- 
> 
> Again, it's a small, simple change that fixes a build break on two 
platforms. 
> 
> Either fix will solve this problem. 
> 
> Best Regards
> 
> Adam Farley 
> 
> > Hi All, 
> > 
> > I ask for a committer to add one word to make/lib/Awt2dLibraries.gmk 
to solve a build break. 
> > 
> > We need to go to line 495 and add array-bounds into the list of 
disabled warnings. 
> > 
> > So this: 
> > 
> > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value, \ 
> > 
> > becomes this: 
> > 
> > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value array-bounds, \ 
> > 
> > This fixes a build-breaking problem which occurs if you don't disable 
> > errors-as-warnings on zLinux or Linux for ppcle. 
> > 
> > Best Regards 
> > 
> > Adam Farley 
> > 
> > P.S. For further background, see this: 
> > http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html 
> 
> 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Thomas Stüfe]

Hi Adam,

this patch looks good. I can sponsor this for you if noone else steps up,
but we need a second reviewer, preferably one from the 2d project.

Best Regards, Thomas

On Wed, Mar 21, 2018 at 12:41 PM, Adam Farley8 
wrote:

> Hi All,
>
> If committers really don't want this code, we could always try fixing the
> code that the warning
> is complaining about.
>
> -- Change --
> -
> diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> --- a/src/java.desktop/share/native/libjavajpeg/jchuff.c
> +++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c
> @@ -805,7 +805,7 @@
>for (i = MAX_CLEN; i > 16; i--) {
>  while (bits[i] > 0) {
>j = i - 2;/* find length of new prefix to be used */
> -  while (bits[j] == 0)
> +  while ((bits[j] == 0) && (j > 0))
>  j--;
>
>bits[i] -= 2; /* remove two symbols */
> -- End of Change --
> -
>
> Again, it's a small, simple change that fixes a build break on two
> platforms.
>
> Either fix will solve this problem.
>
> Best Regards
>
> Adam Farley
>
> > Hi All,
> >
> > I ask for a committer to add one word to make/lib/Awt2dLibraries.gmk to
> solve a build break.
> >
> > We need to go to line 495 and add array-bounds into the list of disabled
> warnings.
> >
> > So this:
> >
> > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value, \
> >
> > becomes this:
> >
> > DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
> shift-negative-value array-bounds, \
> >
> > This fixes a build-breaking problem which occurs if you don't disable
> > errors-as-warnings on zLinux or Linux for ppcle.
> >
> > Best Regards
> >
> > Adam Farley
> >
> > P.S. For further background, see this:
> > http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>

[OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Adam Farley8]

Hi All,

If committers really don't want this code, we could always try fixing the 
code that the warning
is complaining about.

-- Change 
---
diff --git a/src/java.desktop/share/native/libjavajpeg/jchuff.c 
b/src/java.desktop/share/native/libjavajpeg/jchuff.c
--- a/src/java.desktop/share/native/libjavajpeg/jchuff.c
+++ b/src/java.desktop/share/native/libjavajpeg/jchuff.c
@@ -805,7 +805,7 @@
   for (i = MAX_CLEN; i > 16; i--) {
 while (bits[i] > 0) {
   j = i - 2;/* find length of new prefix to be used 
*/
-  while (bits[j] == 0)
+  while ((bits[j] == 0) && (j > 0))
 j--;

   bits[i] -= 2; /* remove two symbols */
-- End of Change 
---

Again, it's a small, simple change that fixes a build break on two 
platforms.

Either fix will solve this problem.

Best Regards

Adam Farley

> Hi All,
> 
> I ask for a committer to add one word to make/lib/Awt2dLibraries.gmk to 
solve a build break.
> 
> We need to go to line 495 and add array-bounds into the list of disabled 
warnings.
> 
> So this:
> 
> DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value, \ 
> 
> becomes this:
> 
> DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough 
shift-negative-value array-bounds, \
> 
> This fixes a build-breaking problem which occurs if you don't disable
> errors-as-warnings on zLinux or Linux for ppcle.
> 
> Best Regards
> 
> Adam Farley
> 
> P.S. For further background, see this:
> http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/008958.html

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type BITMAPV2INFOHEADER & BITMAPV3INFOHEADER is not supported in BMPImageReader

[Jayathirth D V]

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 XRGB.
 
This is why we have redMask = 0x7C00, greenMask = 0x3E0, blueMask = 0x1F in 
case of 16bpp and redMask   = 0x00FF, greenMask = 0xFF00, blueMask  = 
0x00FFfor 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 

Re: [OpenJDK 2D-Dev] [11] RFR JDK-7031957: DIB header of type BITMAPV2INFOHEADER & BITMAPV3INFOHEADER is not supported in BMPImageReader

[Prahalad Kumar Narayanan]

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 XRGB.
 
This is why we have redMask = 0x7C00, greenMask = 0x3E0, blueMask = 0x1F in 
case of 16bpp and redMask   = 0x00FF, greenMask = 0xFF00, blueMask  = 
0x00FFfor 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