JDK-8090038 : HiDPI support may read Screen attributes from the wrong threads
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2013-01-26
  • Updated: 2018-09-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Blocks :  
JDK-8101788 - Support for Hi-DPI displays
Relates :  
JDK-8089779 - Scrolling copyarea optimizations were disabled for HiDPI screens
Relates :  
JDK-8123550 - Severe drawing issues when changing displays
Description
The initial checkin for RT-24009 included 2 references to getting the pixel scaling attribute from the View which are not guaranteed to be executed on the correct thread.  For now, there should be no issues, but as the threading work is finalized in 8.0, we will need to make sure these 2 references are correctly directed into code on the right threads:

- NGNode queries all Screen objects in a static block and records the highest value of pixel scaling in the static variable highestPixelScale.  This value is used from 2 places currently.  Code in NGNode uses this value to disable the scrolling optimizations since they fail for pixel-scaled screens.  Code in NGCanvas uses the value to create a backing store for the canvas that has pixel scaling to match the highest pixel-scaled screen.  If the bug in the scrolling optimizations is fixed then the usage from NGNode could be removed, but NGCanvas will likely always have to match the highest pixel-scaled screen present so that it can be moved to any screen on the system and still render at full resolution.

- ES2SwapChain queries the view of the swap chain to get the current pixel scaling.  This query will have to be moved to the correct code at the start of a rendering pulse.
Comments
Yes, I think this seems reasonable. OK, I will reopen it, lower the priority, and target for 9.
16-11-2013
There are only a few places where we search for the highest pixel scale: Image loading uses it to decide whether to go after @2x images, but recent comments from Apple indicate that @2x works even if the screen isn't a retina display which means we probably should load them regardless - that would remove one use of the screen pixel accessors since that code would become unconditional. Canvas uses it to decide whether to make a scaled back buffer, but that only happens during the render cycle where it is safe to answer the question. CacheFilter uses it, but during the render cycle as well. WebView access the screens as well to determine pixel depth during the PrismGraphicsManager constructor, which probably only happens at render time. If I am correct about my above assumptions, then we only really need to fix the Image loading to solve all issues here...?
15-11-2013
I am ok with reopening it but I don't see how we intend to fix it. Screen initialization must be done in the GUI thread and main() is not the GUI thread. Either main() becomes the GUI thread (too radical) or FX initialization runs before main() (this is what we do now in well known cases). I think that always initializing FX in all applications that use the JDK is a non-starter. Anyhow, reopen. If we are serious about fixing this, then we need to decide the scenarios that we will fix (Swing/SWT) and not code that touches FX before launch() (my fragment and Jim's example). BTW, JUnit tests are broken because they have a main() that is not in a class that is a subclass of Application etc. This means we are not fixing the JUnit case.
15-11-2013
A well-behaved FX application should not touch FX until they call Application.launch, or point the java launcher at a class with no main program. Until that point there is no FX toolkit running. I have no problem telling a developer who does touch FX objects prior to this that they are doing something wrong. Ideally we would do this not only with better documentation, but by better enforcing our thread checks. Where this gets problematic is with Swing or SWT interop interop applications that don't extend from Application at all. In that case, there is no explicit initialization of FX. For the Swing interop case, the first JFXPanel that is constructed will do the initialization. In this case, it would be a reasonable expectation that such an application could construct an Image in the constructor of the Swing app. So this suggests that we do still have a bug here, although it is less serious than other similar bugs (such as the inability to create a WebView node or certain controls off of the FX thread). Rather than filing a new bug, maybe we could reopen this one, lower it to P4 (or P5), and target it for FX 9?
14-11-2013
Steve, so having a different main class than the one that subclasses Application is considered "reaching around public API"? How does a long-running app throw up a FX window only on occasions? It doesn't necessarily want to be launched from an Application class, but it may eventually use an Application class if/when it needs a window. That application may load and resolve a class that has static references to things like Image objects that would trigger the same reaction as Test2, but before it is actually intending to launch an FX window.
14-11-2013
Yes, the fix for the threading problem was to ensure that JFX was initialized before application code could run by detecting that we were launching a JFX application. I never liked this solution much but the alternative (run main() in the UI thread) was too radical a change at the time. Thanks for the attachment.
14-11-2013
The original cases causing the problem were constructing an Image in a static block. I've attached a test case that shows that it seems to work now if the class subclasses from Application, but if you launch from another class, then it can happen. If you run Test then it loads up fine, but if you run Test2 then it does not. Test2 does not use any private API.
14-11-2013
A quick experiment using a well-constructed app with only public APIs did not show the problem I was worried about. So I am OK with leaving this resolved. A new issue can be filed if there is a need.
14-11-2013
Agreed about the "reach around" case, but what I was trying to say above is that I think it might be possible for a well-behaved app to hit this. I will try and post findings here.
14-11-2013
We won't try an fix cases where people reach around public API and touch the toolkit.
14-11-2013
Actually, we should close this. HiDPI support will only read from the wrong thread if FX is accessed private ways. If the real problem is that pixel scale needs to be updated dynamically, let's enter a new bug report to track that issue.
14-11-2013
I agree that we should push this to 9. Note that this may still be possible even for well-behaved applications (that is, apps that inherit from Application and have their main method in that class, or apps that otherwise call Application.launch() before touching anything else in FX) if the first instance of a Node is initialized in the init() method which is guaranteed to run off thread. A simple workaround for this would be to create a node in the constructor of the application. This seems like a P4 issue given that most apps will no longer see this. Thoughts?
14-11-2013
I don't think there is a simple fix for this, we've already fixed as many of the parts of the code that need to know the pixel scale to be as dynamic as possible and to get their information from the appropriate environmental sources that are thread safe. The one remaining access in NGNode is a workaround for only a couple of places that cannot be dynamic and need a more static answer - those are unlikely to be made more dynamic (perhaps one of them could be made more dynamic by 8.0, but not others). I'd recommend pushing this back to 9 when we can consider options to make Canvas and WebView a little more dynamic and/or not need this information.
13-11-2013
There was a long discussion about running main() using the GUI thread and a JIRA that was tracking the discussion. That would make problems like this go away. There may be other ways to fix this but the bottom line is that we are not going to make a change like this for a case that application programmers don't see.
13-11-2013
In normal code, this case is not hit. You can make it happen by running main() outside of an FX Application as follows: import javafx.scene.layout.Region; public class AlertTest_RT27958 { public static void main(String[] args) { new Region().impl_getPeer(); //launch(AlertTest.class, args); } }
13-11-2013
I just ran ContentBoundsTest and still get the error (the code does a try/catch and keeps running).
13-11-2013
I suspect that this is no longer a problem. Need to verify.
30-10-2013
The ES2SwapChain now gets the information from the attributes handed to it by the repainting thread so it is safe. The remaining reference in NGNode is executed in a static initializer and so may still be executed on the wrong thread depending on how the application is started and how and when it resolves its classes.
10-09-2013
Note that reading the Screen information directly in NGNode is also causing unit tests problems since the tests are called without initializing the toolkit and glass native library so they can run headless. [junit] Testcase: testTranslatedRectangle(com.sun.javafx.sg.ContentBoundsTest): Caused an ERROR [junit] null [junit] java.lang.ExceptionInInitializerError [junit] at com.sun.javafx.tk.quantum.QuantumToolkit.createPGRectangle(QuantumToolkit.java:1073) [junit] at javafx.scene.shape.Rectangle.impl_createPGNode(Rectangle.java:383) [junit] at javafx.scene.Node.impl_getPGNode(Node.java:2192) [junit] at com.sun.javafx.sg.ContentBoundsTest.getValidatedPGNode(ContentBoundsTest.java:114) [junit] at com.sun.javafx.sg.ContentBoundsTest.getBounds(ContentBoundsTest.java:130) [junit] at com.sun.javafx.sg.ContentBoundsTest.checkContentPoint(ContentBoundsTest.java:162) [junit] at com.sun.javafx.sg.ContentBoundsTest.checkPoints(ContentBoundsTest.java:249) [junit] at com.sun.javafx.sg.ContentBoundsTest.testTranslatedRectangle(ContentBoundsTest.java:266) [junit] Caused by: java.lang.NullPointerException [junit] at com.sun.glass.ui.Screen.getScreens(Unknown Source) [junit] at com.sun.javafx.sg.prism.NGNode.<clinit>(NGNode.java:69) I will workaround the test failure by adding a try/catch.
26-01-2013