RFR: 8177393: Result of RescaleOp for 4BYTE_ABGR images may be 25% black

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8177393: Result of RescaleOp for 4BYTE_ABGR images may be 25% black

Phil Race
Bug: https://bugs.openjdk.java.net/browse/JDK-8177393
Webrev: http://cr.openjdk.java.net/~prr/8177393/

I have updated the bug report with a long evaluation and some additional
explanation of the proposed fix. I don't intend to duplicate all that here
so please go read it there. The following is a much briefer summary.

We are ending up in medialib native code that cannot interpret the Java 2D child raster
it is handed. The child raster was an attempt to prevent rescaling of the alpha channel.

The proposed fix avoids creating that child raster and instead creates a LookupOp
which has an explicit lookup table for the alpha channel that does the identity lookup.

The fix has a couple of other updates.
(1) the "canUseLookupOp()" method now checks that the DataBuffer is
something that the medialib code used by LookupOp can handle.
This would have fixed the bug by itself although at the expense of always
falling back to slow Java code in RescaleOp. However it is still needed in
case the application itself supplies a child raster.

(2) The slow path in RescaleOp turns out to have the same bug that I think
was seen in some other bug report or fix. It is always re-scaling alpha as
well as colour components. I fixed that too.

Since the fix is entirely confined to RescaleOp any consequences - good or bad - are limited.

I ran the few regression tests we have that cover RescaleOp and also checked Java2Ddemo.

The commented out cases in the new regression test are to be handled under a different bug ID.

-phil.
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8177393: Result of RescaleOp for 4BYTE_ABGR images may be 25% black

prasanta sadhukhan

Looks good to me. [Checked 8080287 also works ok with this change]

Regards
Prasanta
On 5/17/2017 11:14 PM, Phil Race wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8177393
Webrev: http://cr.openjdk.java.net/~prr/8177393/

I have updated the bug report with a long evaluation and some additional
explanation of the proposed fix. I don't intend to duplicate all that here
so please go read it there. The following is a much briefer summary.

We are ending up in medialib native code that cannot interpret the Java 2D child raster
it is handed. The child raster was an attempt to prevent rescaling of the alpha channel.

The proposed fix avoids creating that child raster and instead creates a LookupOp
which has an explicit lookup table for the alpha channel that does the identity lookup.

The fix has a couple of other updates.
(1) the "canUseLookupOp()" method now checks that the DataBuffer is
something that the medialib code used by LookupOp can handle.
This would have fixed the bug by itself although at the expense of always
falling back to slow Java code in RescaleOp. However it is still needed in
case the application itself supplies a child raster.

(2) The slow path in RescaleOp turns out to have the same bug that I think
was seen in some other bug report or fix. It is always re-scaling alpha as
well as colour components. I fixed that too.

Since the fix is entirely confined to RescaleOp any consequences - good or bad - are limited.

I ran the few regression tests we have that cover RescaleOp and also checked Java2Ddemo.

The commented out cases in the new regression test are to be handled under a different bug ID.

-phil.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8177393: Result of RescaleOp for 4BYTE_ABGR images may be 25% black

Phil Race
Some review discussion with Jim is in the bug report comments.

Updated webrev to address the minimal set of issues :
http://cr.openjdk.java.net/~prr/8177393.1/

-phil.

On 5/18/17, 3:32 AM, Prasanta Sadhukhan wrote:

Looks good to me. [Checked 8080287 also works ok with this change]

Regards
Prasanta
On 5/17/2017 11:14 PM, Phil Race wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8177393
Webrev: http://cr.openjdk.java.net/~prr/8177393/

I have updated the bug report with a long evaluation and some additional
explanation of the proposed fix. I don't intend to duplicate all that here
so please go read it there. The following is a much briefer summary.

We are ending up in medialib native code that cannot interpret the Java 2D child raster
it is handed. The child raster was an attempt to prevent rescaling of the alpha channel.

The proposed fix avoids creating that child raster and instead creates a LookupOp
which has an explicit lookup table for the alpha channel that does the identity lookup.

The fix has a couple of other updates.
(1) the "canUseLookupOp()" method now checks that the DataBuffer is
something that the medialib code used by LookupOp can handle.
This would have fixed the bug by itself although at the expense of always
falling back to slow Java code in RescaleOp. However it is still needed in
case the application itself supplies a child raster.

(2) The slow path in RescaleOp turns out to have the same bug that I think
was seen in some other bug report or fix. It is always re-scaling alpha as
well as colour components. I fixed that too.

Since the fix is entirely confined to RescaleOp any consequences - good or bad - are limited.

I ran the few regression tests we have that cover RescaleOp and also checked Java2Ddemo.

The commented out cases in the new regression test are to be handled under a different bug ID.

-phil.