[OpenJDK 2D-Dev] More incompatibilities

[Jim Graham]

This just came through internally for a code review and I looked into it 
in more depth.  Sorry to take so long to get back to everyone, but I 
don't think this is the answer.

I think the problem is in the startRow function.  In the "alphaRows == 
0" case it sets the bbox to x1+1, but in the else part it only expands 
the bbox to x1.  So, startRow is inconsistent with respect to whether it 
expects x1 to be the last pixel in the box, or the first one to the 
right of the box.  It needs to be made consistent and then this code, 
and any other code that calls that function, need to then adhere to that 
convention.  A quick search looks like this is the only place that calls 
it so it shouldn't be too hard to fix by just modifying PiscesCache.

Until startRow is consistent internally, we can't fix this bug by 
changing our convention of how it is called from Renderer...

			...jim

Hiroshi Yamauchi wrote:
> Hi Roman,
> 
> First off, thanks for taking a look at the issues. I confirmed that
> all the tests that I have pass with your patches applied.
> 
> On Tue, Mar 3, 2009 at 2:09 AM, Roman Kennke <roman.kennke at aicas.com> wrote:
>> Hi there,
>>
>>> I believe that the minX and maxX parameters to that function are
>>> relative to the origin of the raster coordinates which means you would
>>> add each of them to the raster's origin (rasterMinX) to get an absolute
>>> min/maxX.  So they look correct as written.
>>>
>>> If this change fixes the problem, then it is a side effect of moving the
>>> right edge of the "startRow" call out to the right some more.  That
>>> could indicate that perhaps maxX was not calculated correctly in the
>>> first place and so expanding it out "covers up" the problem.
>> I had a look at that code. It seems like minX and maxX are calculated
>> correctly. However, in emitRow, we calculate x0 and x1 from that, and
>> then call the cache.startRow() with that to start a new row in the RLE
>> cache. It seems like the cache (or the rendering code that reads the
>> cache) interprets x1 to be the first pixel right of the bounding box,
>> while the emitRow() function needs that pixel _inside_ the bounding box
>> (it does while (srcIdx <= maxX) { .. } ). The following patch fixes the
>> problem for me:
> 
> Aha.
> 
>> diff --git a/src/share/classes/sun/java2d/pisces/Renderer.java
>> b/src/share/classes/sun/java2d/pisces/Renderer.java
>> --- a/src/share/classes/sun/java2d/pisces/Renderer.java
>> +++ b/src/share/classes/sun/java2d/pisces/Renderer.java
>> @@ -634,7 +634,7 @@
>>                 int x0 = minX + (rasterMinX >>
>> SUBPIXEL_LG_POSITIONS_X);
>>                 int x1 = maxX + (rasterMinX >>
>> SUBPIXEL_LG_POSITIONS_X);
>>
>> -                cache.startRow(y, x0, x1);
>> +                cache.startRow(y, x0, x1 + 1);
>>                 int srcIdx = minX;
>>
>>                 // Perform run-length encoding
>> (END)
>>
>> /Roman
>>
>> --
>> Dipl.-Inform. (FH) Roman Kennke, Software Engineer, http://kennke.org
>> aicas Allerton Interworks Computer Automated Systems GmbH
>> Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
>> http://www.aicas.com   * Tel: +49-721-663 968-48
>> USt-Id: DE216375633, Handelsregister HRB 109481, AG Karlsruhe
>> Geschäftsführer: Dr. James J. Hunt
>>
>>
>>
> 
> Hiroshi