[OpenJDK 2D-Dev] [9] Review Request: JDK-8165212 VolatileImage should not be compatible with GraphicsConfiguration which transform is changed

[Sergey Bylokhov]

Looks fine.

On 10.11.16 0:07, Phil Race wrote:
> +1
>
> -phil.
>
> On 11/07/2016 06:17 PM, Prahalad Kumar Narayanan wrote:
>> Hello Everyone on Java2D Group
>>
>> This is a follow-up on the bug fix for JDK-8165212.
>>        Link: https://bugs.openjdk.java.net/browse/JDK-8165212
>>        Title: VolatileImage should not be compatible with
>> GraphicsConfiguration which transform is changed
>>
>> First, Thanks to Alexandr for his review suggestions on webrev.00
>>
>> Root Cause:
>> Upon any change to display's state (Eg: DPI) the callback method-
>> displayChanged() is invoked on VolatileSurfaceManager. The callback
>> method updates the volatile image's surface only if the acceleration
>> is enabled. For non-accelerated volatile images, the surface isn't
>> updated. This caused the bug on OSs where user can change screen's DPI
>> on the fly without having to restart/re-login.
>>
>> Updates to the Fix:
>> In the previous fix version (webrev.00), the software surface was
>> re-created for non-accelerated volatile images as well. This fixed the
>> problem. The suggestion from Alexandr was to re-create the software
>> surface only when the affine transformation of GraphicsConfiguration
>> changes. This is incorporated in the current fix.
>>
>> It is important to note that, the affine transformation of
>> GraphicsConfiguration is actually derived from the screen device. When
>> end-user modifies screen's DPI, the affine transformation is updated
>> on the screen device first; followed by a call to displayChanged()
>> callback on the listeners. Henceforth, a copy of the transformation is
>> kept in the volatile surface manager, to aid in comparing changes to
>> affine transformation (before & after screen's DPI change)
>>
>> Kindly review the changes and provide your suggestions at your
>> convenience.
>> Review link: http://cr.openjdk.java.net/~pnarayanan/8165212/webrev.01/
>>
>> Thank you for your time in review
>> Have a good day
>>
>> Prahalad N.
>> ---
>>
>> From: Alexandr Scherbatiy
>> Sent: Friday, October 28, 2016 4:57 PM
>> To: Prahalad Kumar Narayanan; 2d-dev at openjdk.java.net
>> Subject: Re: [OpenJDK 2D-Dev] [9] Review Request: JDK-8165212
>> VolatileImage should not be compatible with GraphicsConfiguration
>> which transform is changed
>>
>>   350         if (!isAccelerationEnabled()) {
>>   351             // Ideally there is no need to re-create a software
>> surface. But
>>   352             // some OSs allow changes to display state at
>> runtime. Such a
>>   353             // provision would cause mismatch in graphics
>> configuration of the
>>   354             // display and the surface. Hence we re-create the
>> software surface
>>   355             // as well.
>>   356             sdBackup = null;
>>   357             sdCurrent = getBackupSurface();
>>   358         }
>>   359     }
>>
>> It seems that the surface is always recreated when the acceleration is
>> not enabled. May be it should be only done when the graphics
>> configuration transform scales are differ from the ones from vImg
>> graphics configuration?
>>
>> Thanks,
>> Alexandr.
>>   On 10/14/2016 1:38 PM, Prahalad Kumar Narayanan wrote:
>>
>> Hello Everyone
>>   Good day to you.
>>   Request your time in reviewing the fix for-
>>        Bug : JDK-8165212
>>        Title : VolatileImage should not be compatible with
>> GraphicsConfiguration which transform is changed
>>   Description on the bug-
>>         . As per the bug, Volatile Image's graphics configuration is
>> not updated when the host machine display's DPI is changed at runtime
>> (while still running the java app). In addition, the method
>> contentsLost() does not return true when display's DPI is modified.
>>         . It is important to note that, the issue is not reproducible
>> with D3D/OpenGL backend. It is reproducible with non-accelerated
>> Volatile Image.
>>   Root Cause
>>         . A callback method- displayChanged() in
>> VolatileSurfaceManager.java is invoked when display's settings (DPI)
>> is modified.
>>         . The callback method, currently, updates the graphics
>> configuration only for Accelerated volatile image. Graphics
>> configuration is not updated for non-accelerated system memory based
>> VolatileImage.
>>         . Until recently, there wasn't any need for updating graphics
>> configuration for non-accelerated volatileImage. However, Win 8.1 and
>> above provide feature to dynamically update the DPI setting (without
>> requiring for log-off/ log-in), which causes the current bug.
>>   Bug Fix
>>        . First, the callback method is modified to update graphics
>> configuration for non-accelerated volatile image also.
>>        . An update to graphics configuration might require re-creation
>> of the surface. Especially, when the scale factor is increased. Hence
>> the system memory based backupSurface is re-created here.
>>        . The above change is followed by change to Validate() API, so
>> that the backup surface re-creation in displayChanged() method,
>> correctly returns IMAGE_RESTORED from validate() API. This way, the
>> code flow for non-accelerated Volatile Images behaves just the same
>> way as accelerated volatile images.
>>         . Approximately 81 Jtreg test cases (that contained
>> VolatileImage) were run on win7, linux, and osx. No new regressions
>> have been found after the modification.
>>         . In addition, a manual test case has been provided to ensure
>> the proper functioning of the fix
>>   Kindly review the changes and provide your suggestions
>> Review link: http://cr.openjdk.java.net/~pnarayanan/8165212/webrev.00/
>>   Thank you for your time in review
>> Have a good day
>>   Prahalad N.
>


-- 
Best regards, Sergey.