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

[Jim Graham]

On 11/13/14 2:56 PM, Sergey Bylokhov wrote:
> Hi, Jim
>
>> Please revert all "iff => if" changes in Region - those are not typos.
>
> ok
>
>> Hand-coding would require adding a bunch of code for clipping, though
>> it might be faster still by avoiding all of the "are we on a new row?"
>> testing in appendSpan/endRow().
>
> Looks like we get a big drawimage performance improvement from the previous fix - 30%(which is 10000% for the current jdk code) So I will try to go deeper and eliminate the call to appendSpan()/endRow.
>
>>
>> Why does line 272 add "+1" for ?endIndex?  Other places simply use
>> "#rows * 5".
>
> This is because of needSpace() implementation:
> private void needSpace(int num) {
>      if (endIndex + num >= bands.length) { }
> If we have only one row then we need 5 bytes and num will be 5 => equal to the bands.length => unnecessary copyArray will occures.
> I suppose this is because endIndex should be a valid index in the bands array.

That is probably a bug.  That should probably be just ">" since I don't 
see anywhere where we ever place anything in that last entry.

>> Other places in the code use the "clipAdd" and "dimAdd" methods to
>> avoid
>> overflow.  In this particular case we might have overflow for
>> box[0-3].
>>    On the other hand, since appendSpan() clips against the bounds we'd
>
>> resulting spans to be missing or incorrect (though it will never
>> exceed
>> the indicated bounding box even with overflow occuring)".
>
> I do not clip edges intentionally, because I trust the output of TransformHelper, which I assume should validate possible overflow. I'll update a javadoc.

Exactly, this falls more under the realm of "we're creating a new API 
and even though it is only currently used by TransformHelper and we know 
how that works, we should consider that this API might get re-used in 
the future in other cases".  As I said, this code ensures that the 
Region is valid, it just might produce unexpected (i.e. incorrect) 
results if we ever overflow.  TransformHelper is unlikely to ever 
produce values that are much outside of the 0-<couple thousand> range 
since it only generates drawable-bounded coordinates so we should be 
very far from overflow, though what if someone creates a BufferedImage 
that is 1<<30 in dimensions (with the 64-bit VM and a lot of RAM)?

			...jim