[OpenJDK 2D-Dev] [9] Review Request: 8059942 Default implementation of DrawImage.renderImageXform() should be improved for d3d/ogl

[Jim Graham]

Hi Sergey,

On 11/14/14 6:40 AM, Sergey Bylokhov wrote:
> Hi, Jim.
> Please review an updated version of the fix:
> http://cr.openjdk.java.net/~serb/8059942/webrev.17
> Two notes:
>   - one additional byte still is added to the bands, since I am still checking the code where we use endIndex-1/endIndex+1/endIndex++ or swap it with curYrow, etc. This isn't urgent.

Since you no longer call appendSpan(), you no longer need the additional 
byte in this new method.

>   - "end < bands.length" was added to the loop for the reason of overall loop performance(+15%).

That's odd that it helps since you then consume 5 entries after that 
check, but I guess Hotspot works in mysterious ways.  I wonder if 
"end+4" works as well or better since that would be a more accurate test 
of what is happening.  Also, even for 15% performance difference of this 
one method, it might be better to take the AAIOBE as a double check 
against the caller feeding us bad data, such as edges[0,1] that end up 
overflowing?  In either case, we generate valid data even in the face of 
such anomalies.

I'd still prefer to have the code confirm the ranges.  It would still be 
much simpler than the old code that called appendSpan/endRow a lot with 
just a few checks, like this:

     static Region getInstance(final int lox, final int loy, final int hix,
                               final int hiy, final int edges[]) {
// maybe?:        if (hiy <= loy || hix <= lox) return EMPTY_REGION;
         final int y1 = loy + edges[0];
         final int y2 = loy + edges[1];
         // rowsNum * (3 + 1 * 2)
         final int[] bands = new int[(y2 - y1) * 5];
         int end = 0;
         int index = 2;
// Note to Sergey: Does end+4 also generate faster code?
         for (int y = y1; end+4 < bands.length && y < y2; ++y) {
             final int x1 = lox + edges[index++];
             final int x2 = lox + edges[index++];
// Is the following comment too obvious?
             // Note: always consume both indices before rejecting span
             if (y < loy || y >= hiy) continue;
             if (x1 < lox) x1 = lox;
             if (x2 > hix) x2 = hix;
             if (x1 < x2) {
                 bands[end++] = y;     // spanloy
                 bands[end++] = y + 1; // spanhiy
                 bands[end++] = 1;     // 1 span per row
                 bands[end++] = x1;    // spanlox
                 bands[end++] = x2;    // spanhix
             }
         }
         return end != 0 ? new Region(lox, loy, hix, hiy, bands, end)
                         : EMPTY_REGION;
     }

This code should ensure correctness/validity of the Region data 
structure without much performance impact, hopefully?

			...jim