kc3-lang/SDL

diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c
index 596f0a1..355d57b 100644
--- a/src/video/kmsdrm/SDL_kmsdrmopengles.c
+++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c
@@ -61,82 +61,169 @@ KMSDRM_GLES_SwapWindow(_THIS, SDL_Window * window) {
     SDL_DisplayData *dispdata = (SDL_DisplayData *) SDL_GetDisplayForWindow(window)->driverdata;
     SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata);
     KMSDRM_FBInfo *fb_info;
-    int ret, timeout;
+    SDL_bool crtc_setup_pending = SDL_FALSE;
+
+    /* ALWAYS wait for each pageflip to complete before issuing another, vsync or not,
+       or drmModePageFlip() will start returning EBUSY if there are pending pageflips.
+
+       To disable vsync in games, it would be needed to issue async pageflips,
+       and then wait for each pageflip to complete. Since async pageflips complete ASAP 
+       instead of VBLANK, thats how non-vsync screen updates should wok.
+
+       BUT Async pageflips do not work right now because calling drmModePageFlip() with the
+       DRM_MODE_PAGE_FLIP_ASYNC flag returns error on every driver I have tried.
+
+       So, for now, only do vsynced updates: _this->egl_data->egl_swapinterval is
+       ignored for now, it makes no sense to use it until async pageflips work on drm drivers. */
 
     /* Recreate the GBM / EGL surfaces if the display mode has changed */
     if (windata->egl_surface_dirty) {
         KMSDRM_CreateSurfaces(_this, window);
+        /* Do this later, when a fb_id is obtained. */
+        crtc_setup_pending = SDL_TRUE;
     }
 
-    /* Wait for confirmation that the next front buffer has been flipped, at which
-       point the previous front buffer can be released */
-    timeout = 0;
-    if (_this->egl_data->egl_swapinterval == 1) {
-        timeout = -1;
-    }
-    if (!KMSDRM_WaitPageFlip(_this, windata, timeout)) {
-        return 0;
-    }
+    if (windata->double_buffer) {
+        /* Use a double buffering scheme, independently of the number of buffers that the GBM surface has,
+           (number of buffers on the GBM surface depends on the implementation).
+           Double buffering (instead of triple) is achieved by waiting for synchronous pageflip to complete
+           inmediately after the pageflip is issued. That way, in the end of this function only two buffers
+           are needed: a buffer that is available to be picked again by EGL as a backbuffer to draw on it,
+           and the new front buffer that has just been set up.
+
+           Since programmer has no control over the number of buffers of the GBM surface, wait for pageflip
+           is done inmediately after issuing pageflip, and so a double-buffer scheme is achieved. */
+
+        /* Ask EGL to mark the current back buffer to become the next front buffer. 
+           That will happen when a pageflip is issued, and the next vsync arrives (sync flip)
+           or ASAP (async flip). */
+        if (!(_this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface))) {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed.");
+            return 0;
+        }
 
-    /* Release the previous front buffer */
-    if (windata->curr_bo) {
-        KMSDRM_gbm_surface_release_buffer(windata->gs, windata->curr_bo);
-        /* SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Released GBM surface %p", (void *)windata->curr_bo); */
-        windata->curr_bo = NULL;
-    }
+        /* Get a handler to the buffer that is marked to become the next front buffer, and lock it
+           so it can not be chosen by EGL as a back buffer. */
+        windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
+        if (!windata->next_bo) {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock GBM surface front buffer");
+            return 0;
+        /* } else {
+            SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Locked GBM surface %p", (void *)windata->next_bo); */
+        }
 
-    windata->curr_bo = windata->next_bo;
+        /* Issue synchronous pageflip: drmModePageFlip() NEVER blocks, synchronous here means that it
+           will be done on next VBLANK, not ASAP. And return to program loop inmediately. */
 
-    /* Make the current back buffer the next front buffer */
-    if (!(_this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface))) {
-        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed.");
-        return 0;
-    }
+        fb_info = KMSDRM_FBFromBO(_this, windata->next_bo);
+        if (!fb_info) {
+            return 0;
+        }
 
-    /* Lock the next front buffer so it can't be allocated as a back buffer */
-    windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
-    if (!windata->next_bo) {
-        SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock GBM surface front buffer");
-        return 0;
-    /* } else {
-        SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Locked GBM surface %p", (void *)windata->next_bo); */
-    }
+        /* When needed, this is done once we have the needed fb_id, not before. */
+        if (crtc_setup_pending) {
+	    if (KMSDRM_drmModeSetCrtc(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id, 0,
+					0, &dispdata->conn->connector_id, 1, &dispdata->mode)) {
+		SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not configure CRTC on video mode setting.");
+	    }
+            crtc_setup_pending = SDL_FALSE;
+        }
 
-    fb_info = KMSDRM_FBFromBO(_this, windata->next_bo);
-    if (!fb_info) {
-        return 0;
-    }
+        if (!KMSDRM_drmModePageFlip(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id,
+                                    DRM_MODE_PAGE_FLIP_EVENT, &windata->waiting_for_flip)) {
+            windata->waiting_for_flip = SDL_TRUE;
+        } else {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not issue pageflip");
+        }
 
-    if (!windata->curr_bo) {
-        /* On the first swap, immediately present the new front buffer. Before
-           drmModePageFlip can be used the CRTC has to be configured to use
-           the current connector and mode with drmModeSetCrtc */
-        ret = KMSDRM_drmModeSetCrtc(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id, 0,
-                                    0, &dispdata->conn->connector_id, 1, &dispdata->mode);
+        /* Since issued pageflips are always synchronous (ASYNC dont currently work), these pageflips
+           will happen at next vsync, so in practice waiting for vsync is being done here. */ 
+        if (!KMSDRM_WaitPageFlip(_this, windata, -1)) {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Error waiting for pageflip event");
+            return 0;
+        }
 
-        if (ret) {
-          SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not configure CRTC");
+        /* Return the previous front buffer to the available buffer pool of the GBM surface,
+           so it can be chosen again by EGL as the back buffer for drawing into it. */
+        if (windata->curr_bo) {
+            KMSDRM_gbm_surface_release_buffer(windata->gs, windata->curr_bo);
+            /* SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Released GBM surface buffer %p", (void *)windata->curr_bo); */
+            windata->curr_bo = NULL;
         }
+
+        /* Take note of the current front buffer, so it can be freed next time this function is called. */
+        windata->curr_bo = windata->next_bo;
     } else {
-        /* On subsequent swaps, queue the new front buffer to be flipped during
-           the next vertical blank */
-        ret = KMSDRM_drmModePageFlip(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id,
-                                     DRM_MODE_PAGE_FLIP_EVENT, &windata->waiting_for_flip);
-        /* SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "drmModePageFlip(%d, %u, %u, DRM_MODE_PAGE_FLIP_EVENT, &windata->waiting_for_flip)",
-            viddata->drm_fd, displaydata->crtc_id, fb_info->fb_id); */
-
-        if (_this->egl_data->egl_swapinterval == 1) {
-            if (ret == 0) {
-                windata->waiting_for_flip = SDL_TRUE;
-            } else {
-                SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not queue pageflip: %d", ret);
-            }
+        /* Triple buffering requires waiting for last pageflip upon entering instead of waiting at the end,
+           and issuing the next pageflip at the end, thus allowing the program loop to run 
+           while the issued pageflip arrives (at next VBLANK, since ONLY synchronous pageflips are possible).
+           In a game context, this means that the player can be doing inputs before seeing the last
+           completed frame, causing "input lag" that is known to plage other APIs and backends.
+           Triple buffering requires the use of three different buffers at the end of this function:
+           1- the front buffer which is on screen,
+           2- the back buffer wich is ready to be flipped (a pageflip has been issued on it, which has yet to complete)
+           3- a third buffer that can be used by EGL to draw while the previously issued pageflip arrives
+              (should not put back the previous front buffer into the free buffers pool of the
+              GBM surface until that happens).
+           If the implementation only has two buffers for the GBM surface, this would behave like a double buffer.
+        */
+        
+        /* Wait for previously issued pageflip to complete. */
+        if (!KMSDRM_WaitPageFlip(_this, windata, -1)) {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Error waiting for pageflip event");
+            return 0;
+        }
+
+        /* Free the previous front buffer so EGL can pick it again as back buffer.*/
+        if (windata->curr_bo) {
+            KMSDRM_gbm_surface_release_buffer(windata->gs, windata->curr_bo);
+            /* SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Released GBM surface buffer %p", (void *)windata->curr_bo); */
+            windata->curr_bo = NULL;
+        }
+
+        /* Ask EGL to mark the current back buffer to become the next front buffer. 
+           That will happen when a pageflip is issued, and the next vsync arrives (sync flip)
+           or ASAP (async flip). */
+        if (!(_this->egl_data->eglSwapBuffers(_this->egl_data->egl_display, windata->egl_surface))) {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "eglSwapBuffers failed.");
+            return 0;
         }
 
-        /* Wait immediately for vsync (as if we only had two buffers), for low input-lag scenarios.
-           Run your SDL2 program with "SDL_KMSDRM_DOUBLE_BUFFER=1 <program_name>" to enable this. */
-        if (_this->egl_data->egl_swapinterval == 1 && windata->double_buffer) {
-            KMSDRM_WaitPageFlip(_this, windata, -1);
+        /* Take note of the current front buffer, so it can be freed next time this function is called. */
+        windata->curr_bo = windata->next_bo;
+
+        /* Get a handler to the buffer that is marked to become the next front buffer, and lock it
+           so it can not be chosen by EGL as a back buffer. */
+        windata->next_bo = KMSDRM_gbm_surface_lock_front_buffer(windata->gs);
+        if (!windata->next_bo) {
+             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not lock GBM surface front buffer");
+             return 0;
+	/* } else {
+             SDL_LogDebug(SDL_LOG_CATEGORY_VIDEO, "Locked GBM surface %p", (void *)windata->next_bo); */
+        }
+
+        /* Issue synchronous pageflip: drmModePageFlip() NEVER blocks, synchronous here means that it
+           will be done on next VBLANK, not ASAP. And return to program loop inmediately. */
+        fb_info = KMSDRM_FBFromBO(_this, windata->next_bo);
+        if (!fb_info) {
+            return 0;
+        }
+
+        /* When needed, this is done once we have the needed fb_id, not before. */
+        if (crtc_setup_pending) {
+	    if (KMSDRM_drmModeSetCrtc(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id, 0,
+					0, &dispdata->conn->connector_id, 1, &dispdata->mode)) {
+		SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not configure CRTC on video mode setting.");
+	    }
+            crtc_setup_pending = SDL_FALSE;
+        }
+
+
+        if (!KMSDRM_drmModePageFlip(viddata->drm_fd, dispdata->crtc_id, fb_info->fb_id,
+                                    DRM_MODE_PAGE_FLIP_EVENT, &windata->waiting_for_flip)) {
+            windata->waiting_for_flip = SDL_TRUE;
+        } else {
+            SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not issue pageflip");
         }
     }
 
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index 383170b..608bee5 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -188,7 +188,7 @@ KMSDRM_CreateDevice(int devindex)
 
     device->driverdata = viddata;
 
-    /* Setup all functions which we can handle */
+    /* Setup all functions that can be handled from this backend. */
     device->VideoInit = KMSDRM_VideoInit;
     device->VideoQuit = KMSDRM_VideoQuit;
     device->GetDisplayModes = KMSDRM_GetDisplayModes;
@@ -303,6 +303,12 @@ KMSDRM_FBFromBO(_THIS, struct gbm_bo *bo)
 static void
 KMSDRM_FlipHandler(int fd, unsigned int frame, unsigned int sec, unsigned int usec, void *data)
 {
+    /* If the data pointer received here is the same passed as the user_data in drmModePageFlip()
+       then this is the event handler for the pageflip that was issued on drmPageFlip(): got here 
+       because of that precise page flip, the while loop gets broken here because of the right event.
+       This knowledge will allow handling different issued pageflips if sometime in the future 
+       managing different CRTCs in SDL2 is needed, for example (synchronous pageflips happen on vblank 
+       and vblank is a CRTC thing). */
     *((SDL_bool *) data) = SDL_FALSE;
 }
 
@@ -331,8 +337,12 @@ KMSDRM_WaitPageFlip(_THIS, SDL_WindowData *windata, int timeout) {
             return SDL_FALSE;
         }
 
+        /* Is the fd readable? Thats enough to call drmHandleEvent() on it. */
         if (pfd.revents & POLLIN) {
-            /* Page flip? If so, drmHandleEvent will unset windata->waiting_for_flip */
+            /* Page flip? ONLY if the event that made the fd readable (=POLLIN state)
+               is a page flip, will drmHandleEvent call page_flip_handler, which will break the loop.
+               The drmHandleEvent() and subsequent page_flip_handler calls are both synchronous (blocking),
+               nothing runs on a different thread, so no need to protect waiting_for_flip access with mutexes. */
             KMSDRM_drmHandleEvent(viddata->drm_fd, &ev);
         } else {
             /* Timed out and page flip didn't happen */
@@ -776,7 +786,7 @@ KMSDRM_CreateWindow(_THIS, SDL_Window * window)
     /* Maybe you didn't ask for a fullscreen OpenGL window, but that's what you get */
     window->flags |= (SDL_WINDOW_FULLSCREEN | SDL_WINDOW_OPENGL);
 
-    /* In case we want low-latency, double-buffer video, we take note here */
+    /* In case low-latency is wanted, double-buffered video will be used. We take note here */
     windata->double_buffer = SDL_FALSE;
 
     if (SDL_GetHintBoolean(SDL_HINT_VIDEO_DOUBLE_BUFFER, SDL_FALSE)) {