Skip to content

Conversation

@tsayao
Copy link
Collaborator

@tsayao tsayao commented Oct 4, 2025

As Michael Zucchi pointed out on the mailing list, the high framerate occurs because glXSwapBuffers() operates asynchronously. To ensure proper synchronization, you can call glFinish() afterward, which blocks until the buffer swap is fully completed. However, when using glXSwapIntervalSGI, the swap interval setting applies globally rather than per drawable. In contrast, glXSwapIntervalEXT provides per-drawable control, allowing finer-grained vsync behavior.

I don't know if there are scenarios when the unlimited frame rate is needed - if so we should provide a option.

See https://wikis.khronos.org/opengl/Swap_Interval

It also selects the correct visual for transparency which needs to be depth = 32 for X11.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

  • JDK-8210547: [Linux] Uncontrolled framerate (Bug - P5)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1929/head:pull/1929
$ git checkout pull/1929

Update a local copy of the PR:
$ git checkout pull/1929
$ git pull https://git.openjdk.org/jfx.git pull/1929/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1929

View PR using the GUI difftool:
$ git pr show -t 1929

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1929.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 4, 2025

👋 Welcome back tsayao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Oct 4, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot added the rfr Ready for review label Oct 4, 2025
@mlbridge
Copy link

mlbridge bot commented Oct 4, 2025

Webrevs

@kevinrushforth
Copy link
Member

Reviewers: @kevinrushforth @lukostyra

/reviewers 2

@openjdk
Copy link

openjdk bot commented Oct 4, 2025

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

@tsayao
Copy link
Collaborator Author

tsayao commented Oct 4, 2025

I'm not sure of the solution, but it's a starting point.

@kevinrushforth
Copy link
Member

@arapte If you could also take a look that would be helpful.

@notzed
Copy link

notzed commented Oct 9, 2025

Nice, I (Michael Z) can confirm it fixes the frame throttling here, although in a simple animation all rendered frames have a pulse-presentation latency of 33ms -- which is somewhat better than the 48ms previously.

I've been doing some testing of the pulse-presentation latency, taking timestamps of the start of the pulse, the end of the render (before glXSwapBuffers), and the GL_TIMESTAMP of when glXSwapBuffers() is processed (via glQueryCounter()). Plus tracking the pulse number all the way through so they can be matched. This is some of the output with the patch:

event                               pulse-submit   present

pulse 1050 17484397796
present 1048 17467793037 17484194371  -   +16.697  +33.098
pulse 1051 17501064359
present 1049 17484490371 17500860741  -   +16.782  +33.152
pulse 1052 17517726363
present 1050 17501155852 17517524593  -   +16.758  +33.127
pulse 1053 17534403585

(This is in logging order, not temporal)

As a comparison using the existing SwapIntervalSGI and calling glFinish() after all scenes have rendered results in a pulse-presentation latency of 16.7ms instead. Adding the glFinish() to the SwapIntervalEXT version has no effect.

pulse 1050 17478236976
present 1048 17461465333 17461674222  -   +16.569  +16.778
pulse 1051 17494927765
present 1049 17478075556 17478273482  -   +16.443  +16.641
pulse 1052 17511531058
present 1050 17494771852 17494973482  -   +16.535  +16.737
pulse 1053 17528240031

??? I don't know why they'd be different given they both have the same order of pulses, but the timing code is the same.

For the curious, I blogged about the timings I did yesterday, I haven't added any plots with this patch but they look the same as "Build 2" but with another frame of latency to the present times (around 33 vs 16-17ms).

I don't know if there are scenarios when the unlimited frame rate is needed - if so we should provide a option.

Well if prism.vsync=false it wont call swapinterval and will just run at the pulse rate, plus there's javafx.animation.fullspeed?

@notzed
Copy link

notzed commented Oct 9, 2025

Sorry I was wrong and should've tested with more than 1 window. The change to glXSwapIntervalEXT() doesn't work properly and behaves as if glFinish() is called for each window after glXSwapBuffer(), each causing a new wait for vblank.

@lukostyra
Copy link
Contributor

I don't know if there are scenarios when the unlimited frame rate is needed - if so we should provide a option.

Well if prism.vsync=false it wont call swapinterval and will just run at the pulse rate, plus there's javafx.animation.fullspeed?

javafx.animation.fullspeed=true has the same effect as prism.vsync=false and more. Both disable vsync in order to uncap the framerate from monitor's refresh rate, and in addition to that, javafx.animation.fullspeed=true also sets the pulse timer to immediately trigger the next pulse.

While not applicable during day-to-day JavaFX use, unlimited framerate can actually be quite useful. We currently actively use javafx.animation.fullspeed=true when working on Metal and D3D12 to do performance and stability testing. It would be good to keep it around and functional on Linux as well.

@kevinrushforth kevinrushforth self-requested a review October 10, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rfr Ready for review

Development

Successfully merging this pull request may close these issues.

4 participants