Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphics: Add Sdl2 vsync #4854

Merged
merged 5 commits into from Dec 10, 2020
Merged

Graphics: Add Sdl2 vsync #4854

merged 5 commits into from Dec 10, 2020

Conversation

matham
Copy link
Member

@matham matham commented Dec 29, 2016

Continued from #4120.

@dessant
Copy link
Contributor

dessant commented Jan 3, 2017

@matham, fately?

Edit: I mean what will be the fate of this? 😛

@matham
Copy link
Member Author

matham commented Jan 3, 2017

In my tests I could never get it to work. Either there was tearing despite these options, or there never was tearing. However, I tested it on rpi and another higher power system. It should be tested on a normal system that has tearing to see if this fixes it.

@ignus2
Copy link
Contributor

ignus2 commented Jan 7, 2017

The way this implements vsync should work, I have yet to test it, but I'm using the same function call in my app (SDL_GL_SetSwapInterval) just through pysdl2 and (yes, it's a hack but) it works.

However, I highly recommend NOT to use a boolean for this setting. There is a reason the SDL function is called "SwapInterval" and not "SetVsyncOnOff" or whatever. There is a legitimate use-case for setting the swap interval to 2 or even higher. Not widely used, I know, but it has it's place of use.

I propose to change the setting to an integer and also rename it to vsyncinterval or swapinterval or something similar.

@psederberg
Copy link

Just chiming in here as another developer making use of kivy (thank you for a great library, BTW).

I second the suggestion for calling this swapinterval and having it be an integer. My use case requires specifically setting it to 1 and not -1, even if the GPU/driver supports it, which the current PR would not allow.

I would love it if I didn't have to worry about pysdl2 anymore once this new method is added and would be happy to test it out if that would help.

@dessant dessant added the Status: Incomplete Issue/PR is incomplete, but it's real label Feb 26, 2017
@ghost
Copy link

ghost commented Mar 10, 2017

Guys, I think I have something interesting.

After adding the calls to SDL api at the very end, I've got it to work. Debug says that on Ubuntu and Android swap interval == 1 works. Before I've tried the code from the PR and SDL_GetError() said that no GL context currently has been set up. Maybe all thing needed was to delay the call? I dont know if I missed something more advanced, but it seems it finally works.

def setup_window(*args):
    #blabla

    # Open all available joysticks
    cdef int joy_i
    for joy_i in range(SDL_NumJoysticks()):
        SDL_JoystickOpen(joy_i)

    SDL_SetEventFilter(_event_filter, <void *>self)

    SDL_EventState(SDL_DROPFILE, SDL_ENABLE)
    cdef int w, h
    SDL_GetWindowSize(self.win, &w, &h)

    result = SDL_GL_SetSwapInterval(-1)
    if result == -1:
        print('sdl2 does not support late swap tearing')
        print SDL_GetError()
        result = SDL_GL_SetSwapInterval(1)
        if result == -1:
            print('sdl2 does not support vsync with interval=1')
            print SDL_GetError()
        else:
            print('sdl2 vsync interval=1 on')
    else:
        print('sdl2 vsync -1 (late swap tearing) on')

    return w, h

Sorry for code quality as its cut directly from test script, but I believe it shows the idea.

@kivy/core-developers what do you think on that one?

Cheers

@matham
Copy link
Member Author

matham commented Mar 10, 2017

I didn't have any gl context issues when I tested it. The issues we had were with not being sure whether the PR had any effect on sync. The code executed without any issues. I just couldn't tell whether the graphics actually behaved differently (it generally didn't).

@ghost
Copy link

ghost commented Mar 10, 2017

:( I hope someone will figure it out. Thanks for clarifying.

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

Setting vsync does make a difference. If you let the app free-run (with maxfps at 0), vsync should limit the update correctly, but only if there is graphics update (like continuous video playback for example).
You can test without video playback by enabling vsync, setting maxfps to 0, and scheduling a forced redraw every frame by:
Clock.schedule_interval(lambda t: Window.canvas.ask_update(), 0)
You should have the framerate capped at vsync this way.

@ghost
Copy link

ghost commented Mar 21, 2017

@ignus2 doesnt it makes the performance worse (maxfps=0)?

@ghost
Copy link

ghost commented Mar 21, 2017

And I wonder how it works for animations? Graphics tearing during horizontal animations is horrible, and it can easily be seen in for example ScreenManager.

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

maxfps=0 disables kivy's clock sleep between frames, and runs as fast as it can. It's not really a tearing issue necessarily, I make heavy use of vsync for displaying jitter-free 30fps video and some animations on 30Hz monitors, and I rely on vsync to do the scheduling for me. It works perfectly.
Though I currently have to use a pysdl hack and call SDL_GL_SetSwapInterval directly, as kivy doesn't support setting the swap interval yet.

@ghost
Copy link

ghost commented Mar 21, 2017

Could you maybe post example Kivy code with pysdl2 hack used? I wonder from which part of the app and when youre calling it. I would very appreciate your help :)

Why do you say vsync has nothing to do with tearing? Afaik reason for tearing to happen is the lack of the sync between frames redraw...

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

# NOTE: importing SDL2.dll must be done before kivy import, otherwise pysdl2 croaks
# This is Windows-only for now
import os, sys
sdl2_dll_dir = os.path.join(sys.prefix, 'share', 'sdl2', 'bin')
os.environ['PYSDL2_DLL_PATH'] = sdl2_dll_dir if os.path.isdir(sdl2_dll_dir) else os.path.dirname(os.path.abspath(__file__))
import sdl2

# ... then when kivy starts up and you have a window:
sdl2.SDL_GL_SetSwapInterval(1)

I'm not sure if setting the DLL path is really needed or not for just SetSwapInterval, I also import some other functions for debugging from SDL2 dll directly...

I wrote it's not really a tearing issue necessarily (in my case for example), not that it has nothing to do with tearing. It has other purposes apart from just tearing.

@ghost
Copy link

ghost commented Mar 21, 2017

@ignus2 I wonder, "Windows only" is the need to import dll, not the whole hack?

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

I tested only on Windows. But why don't you try it yourself?

@ghost
Copy link

ghost commented Mar 21, 2017

Of course I tested it myself, I was just curious if it's some "windows-only" thing. I cant see any improvement though, tearing is still there (Ubuntu, Android).

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

Tearing on Linux is a whole different story, unrelated to kivy and SDL... Good luck getting rid of it :)
EDIT: Yes, it's Windows-only, however, it's important to understand that it probably should work on Linux too if all other stuff with the window/desktop manager, compositor and driver is set up and works correctly. And that is not easy to get right, I could never get it work, either because of some driver issue, or something else.

@ghost
Copy link

ghost commented Mar 21, 2017

My main target is still Android and it hurts the eyes to see the tearing during animations. Kivy has fantastic Animations API and ScreenManager for different views, but its sad when using them this is common to see visual artifacts.

Back to the topic, it seems that sdl2 has the bug related to vsync.

@ignus2
Copy link
Contributor

ignus2 commented Mar 21, 2017

You should google for tearing on Android using SDL2 in general then.

@jabdoa2
Copy link
Contributor

jabdoa2 commented May 1, 2018

Any chance to get this merged?

@ignus2
Copy link
Contributor

ignus2 commented May 2, 2018

It would be nice, but I'd definitely like to have the vsync setting changed from a boolean to an integer, and set it directly as the swapinterval. Also, the option should be renamed to vsyncinterval or swapinterval in that case.

@matham matham closed this Mar 7, 2019
@matham matham changed the title Add Sdl2 vsync [graphics]Add Sdl2 vsync May 23, 2019
@matham matham reopened this Oct 28, 2020
@psederberg
Copy link

Hi @matham, thanks for updating this PR for the upcoming release! We tested it on OSX and it did not seem to have any effect, however, if we move the code in kivy/core/window/_window_sdl.pyx where you call SDL_GL_SetSwapInterval to after the window is created (we just put it right before the return of that setup_window method) then it did seem to work (i.e., setting vsync to 0 did disable it and setting it to 1 turned it back on).

We also slightly modified the code so that it makes use of the vsync int, itself, because, as someone noted above, there are some valid use cases where someone may want to set vsync to a value greater than one (e.g., 2), which is technically allowed by the OpenGL spec. Note, that on OSX it seems that values greater than 1 are ignored.

It may be that you'd want to update the logic to be that anytime vsync is set to something other than 1 it will fall back to setting it to 1 upon failure (not just for the vsync=-1 case).

Thanks again and we're happy to test more changes!

@matham
Copy link
Member Author

matham commented Nov 4, 2020

I updated the PR with the suggested changes.

@psederberg
Copy link

Thanks @matham! We tested it on both OSX and Linux and everything worked as expected. Setting vsync to 0 turned it off, setting it to 1 turned it on (running at 60 Hz), and (on Linux, but not OSX) setting it to 2 throttled it to 30 Hz. I think OSX simply doesn't accept setting the swap interval above 1.

Note, we didn't have a good way to test Windows, nor a good way to test adaptive vsync by setting it to -1. It is the case that it did not throw an error when we tried setting vsync to -1 and it did correctly run our app at 60 Hz, so it's likely working and we just couldn't see any different behavior in our app.

@matham
Copy link
Member Author

matham commented Nov 5, 2020

Ok, perfect. I'll merge this once we release 2.0.0.

Seems the tests are failing...sigh... I don't see that we did anything that should cause the test failures. I guess we'll have to figure it out after the release.

@matham matham added Component: graphics kivy/graphics and removed Status: Incomplete Issue/PR is incomplete, but it's real labels Dec 10, 2020
@matham matham added this to the 2.1.0 milestone Dec 10, 2020
@matham matham changed the title [graphics]Add Sdl2 vsync Graphics: Add Sdl2 vsync Dec 10, 2020
@matham matham merged commit 0b7b6a1 into master Dec 10, 2020
@matham matham deleted the sdl2_vsync branch December 10, 2020 04:02
@matham
Copy link
Member Author

matham commented Feb 24, 2021

I finally tested this and was able to see that it is indeed working.

Here's is the test script and results showing that setting vsync does control whether the GPU blocks to synchronize with the frames. It shows for each frame; the time kivy processes the clock callbacks, the time before frame flipping, and the time after frame flipping. It's all relative to the time the last frame finished flipping.

I'm posting it for future reference for myself as a good test to see if frames are being dropped.

vsync

from multiprocessing import Process, Queue

from kivy.config import Config
from kivy.uix.widget import Widget
from kivy.app import App
from kivy.clock import Clock

import numpy as np
import matplotlib.pyplot as plt
from time import perf_counter, sleep

FPS = 60


class MyApp(App):

    values = {}
    values_list = []
    count = 0
    tick_event = None

    def build(self):
        return Widget()

    def on_start(self):
        from kivy.core.window import Window
        Clock._max_fps = 0
        Window.fbind('on_flip', self.flip_callback)

        self.tick_event = Clock.create_trigger(
            self.tick_callback, 0, interval=True)
        Clock.schedule_once(self.tick_event, .25)

    def tick_callback(self, *largs):
        self.count += 1
        if self.count == 15 * FPS:
            self.tick_event.cancel()
            self.stop()
            return

        self.values[self.count] = [perf_counter()]
        sleep(0)
        self.root.canvas.ask_update()

    def flip_callback(self, *largs):
        '''Called before every GPU frame by the graphics system.
        '''
        from kivy.core.window import Window
        ts = perf_counter()
        Window.on_flip()
        te = perf_counter()
        if self.count in self.values:
            self.values[self.count].append(ts)
            self.values[self.count].append(te)
            self.values_list.append(self.values[self.count])
        return True


def run_kivy(queue, option):
    Config.set('graphics', 'vsync', option)
    app = MyApp()
    app.run()
    queue.put(app.values_list)


if __name__ == '__main__':
    options = '-1', '0', '1', '2'
    results = []
    for option in options:
        queue = Queue()
        p = Process(target=run_kivy, args=(queue, option))
        p.start()
        results.append(queue.get())
        p.join()

    fig, axs = plt.subplots(2, 2, sharey='row', sharex='col')
    for i, (values_list, option) in enumerate(zip(results, options)):
        ax = axs[i // 2, i % 2]

        values = np.asarray(values_list)
        base = values[:-1, 2]
        ax.plot(values[1:, 0] - base, '.', label='tick' if i == 1 else None)
        ax.plot(values[1:, 1] - base, '.', label='pre-flip' if i == 1 else None)
        ax.plot(values[1:, 2] - base, '.', label='post-flip' if i == 1 else None)
        ax.set_title(f'vsync={option}')
        if not (i % 2):
            ax.set_ylabel('sec')
        if i // 2:
            ax.set_xlabel('Frame #')
        if i == 1:
            ax.legend()

    plt.show()

hamlet4401 pushed a commit to tytgatlieven/kivy that referenced this pull request Jul 3, 2021
* Add support for vsync configuration.

* Accept all integers.

* Bump config version.

* The added version is now 2.1.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: graphics kivy/graphics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants