RFR: 8274932: Render scales in EmbeddedWindow are not properly updated [v6]

Prasanta Sadhukhan psadhukhan at openjdk.org
Wed Aug 2 04:27:31 UTC 2023


On Tue, 1 Aug 2023 10:25:33 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> @prsadhuk @hjohn I'll take a closer look early next week, but I think the best way forward might be for Prasanta to evaluate #1189 and, if the modified fix in that PR is what we want to use, update _this_ PR to include the change that adds the call to `updateSceneState()` and removes the explicit call to `Stage::setRenderScale[XY]`. Then add John as a contributor to this PR using `/contributor add`. 

I have tested both JDK-8222209 and JDK-8274932 with #1189 fix and it works fine. 
Only question I have is, should `updateSceneState` be called in `QuantumToolkit.runWithRenderLock` synchronization?
And also should we move `updateSceneState` to `GlassScene.entireSceneNeedsRepaint` as that changes the scene, so scene state needs to be updated!!

> We can then close [JDK-8222209](https://bugs.openjdk.org/browse/JDK-8222209) as a duplicate of [JDK-8274932](https://bugs.openjdk.org/browse/JDK-8274932).

I think JDK-8274932 is about scale correction and JDK-8222209 is about content refresh in different-scaled configuration so I dont think one is duplicate of another. I can add #1189 change to this PR and do /contributor add but I think we should do "/issue add JDK-8222209" instead of closing that as duplicate. 
[Another possibility is we can have this PR only contain swing-interop change and let #1189 having only non swing-interop updateSceneState change and assign JDK-8222209 to @hjohn]

> > > @prsadhuk @hjohn I'll take a closer look early next week, but I think the best way forward might be for Prasanta to evaluate #1189 and, if the modified fix in that PR is what we want to use, update _this_ PR to include the change that adds the call to `updateSceneState()` and removes the explicit call to `Stage::setRenderScale[XY]`. Then add John as a contributor to this PR using `/contributor add`.
> > 
> > 
> > I have tested both JDK-8222209 and JDK-8274932 with #1189 fix and it works fine. Only question I have is, should `updateSceneState` be called in `QuantumToolkit.runWithRenderLock` synchronization? And also should we move `updateSceneState` to `GlassScene.entireSceneNeedsRepaint` as that changes the scene, so scene state needs to be updated!!
> 
> I'm not sure, we'd have to look into that further still (is the lock also required for calling `entireSceneNeedsRepaint`?)
> 
> It's possible the lock is there because that `WindowEvent` is received on a non-FX thread.

I will let windows-toolkit person to comment on the requirement of lock..I think it might be needed...



> My primary concern is that https://github.com/openjdk/jfx/pull/1171 is doing Platform.runLater calls that I think are undesired and unnecessary if you add updateSceneState. In other words, the fix in https://github.com/openjdk/jfx/pull/1189 might fix both problems without needing to call Platform.runLater, but I haven't tested that.

That anyway I would have deleted when #1189 would have been approved...anyway, I have merged that PR in this...

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1171#issuecomment-1661469846


More information about the openjfx-dev mailing list