[OpenJDK 2D-Dev] Review Request: Small tweak in the DrawImage.java

[Sergey Bylokhov]

On 12.11.15 23:46, Jim Graham wrote:
> Hi Sergey,
>
> I think the added logic is fine, but I would hesitate removing all of
> those lines with comments simply because:
>
> - In terms of code generated it is no more efficient to do the tests up
> above or down below.
>
> - Your question below does not indicate enough investigation to remove
> that big long reminder, even if it is true that other pipelines need to
> have similar tests.  Keep in mind that this is a reminder of why not to
> remove those checks even if they look like they don't really help
> anything and the amount of work it would take to eliminate them should
> we want to do that in the future...

Actually my intention was to create the small and fast list of checks, 
which allow us totally to skip all Blits machinery.
- We will save a few time on blit lookup.
- Most of the OGL/d3d blits have such checks, but they are implemented 
in the native code which will be executed after we posted the necessary 
operations to the OGLRenderQueue. For example take a look to the 
OGLBlitLoops.IsoBlit(). This method uses rq.lock(), then calls 
validateContext() which can be extremely slow if the clip is not 
rectangle(see BufferedContext.setClip()), and then this method calls 
rq.flushNow()!!!
Even if such operations in the OGLBlitLoops.IsoBlit() for particular 
destination will be fast, they will change the current OGL context. This 
can slowdown rendering to another destination, because another blit 
should restore the context. The possible solution for such issues is to 
add these checks to the java blits in OGLBlitLoops and to related blits 
in d3d. But I assume that DrawImage.blitSurfaceData is a better place. 
This is why I also requested some other fast checks which can speedup 
us, like clipRegion.intersectsQuickCheckXYXY(dx,dy,dx+w,dy+h)

I can add a comment that this code is for performance, what do you think?


>
>              ...jim
>
> On 11/12/2015 5:45 AM, Sergey Bylokhov wrote:
>> Hello.
>> Can somebody take a look to the small enhancement for DrawImage.java.
>>
>> I will file a new CR if the proposed change will be approved:
>> http://cr.openjdk.java.net/~serb/perf/DrawImageItself.01/src/java.desktop/share/classes/sun/java2d/pipe/DrawImage.java.sdiff.html
>>
>>
>>
>> The goal is to minimize the calls to Blit.Blit/BlitBG() if we know that
>> it will be noop.
>> The reasons to the fix:
>>   - Not all blits are fast when the source and destination are the same.
>> For example ByteBinary variants are quite slow.
>>   - In case of d3d and ogl such blit change the whole state of the
>> render context and can slowdown the rendering to another destination.
>>
>> Notes:
>>   - In the common situation one additional statement "srcData ==
>> dstData" will be executed.
>>   - The comment about the 4783274 was removed, because I am not sure we
>> really should copy this check from this particular method to all java
>> blits?
>>   - Probably some other checks can be added as well? What about
>> clipRegion.intersectsQuickCheckXYXY(dx,dy,dx+w,dy+h)?
>>
>>


-- 
Best regards, Sergey.