[EXPERIMENTAL] RetroPlayer: OpenGL hardware rendering for game clients - #151
Open
sunlollyking wants to merge 26 commits into
Open
[EXPERIMENTAL] RetroPlayer: OpenGL hardware rendering for game clients#151sunlollyking wants to merge 26 commits into
sunlollyking wants to merge 26 commits into
Conversation
…ndle hw framebuffers
The FBO buffer/pool and renderer were added to both the OpenGl and OpenGLES blocks, which double-adds them when a build enables both, and were not gated on EGL at all. CRenderBufferPoolFBO creates a shared EGL context off CWinSystemEGL, so it needs the same gbm/wayland + EGL guard the DMA path already uses -- a bare OpenGl guard breaks Windows/OSX. Gate on GL only for now; GLES is out of scope. CRenderBufferFBO::CreateTexture() also requested a GL_LINEAR_MIPMAP_LINEAR min filter and called glGenerateMipmap() before glTexImage2D() had allocated level 0. Only level 0 is ever allocated and the core redraws it every frame, so the texture was mipmap-incomplete and would sample black. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Registers CRendererFactoryFBO on the GBM and Wayland desktop-GL window systems. The original WIP registered it on Wayland only and worked around pool selection by commenting out the DMA renderer; this does the selection properly instead, so software cores keep using DMA and sysmem. Selection is driven by the stream's pixel format. Hardware-rendered streams carry no CPU-side format, so CRenderBufferPoolFBO::ConfigureInternal accepts AV_PIX_FMT_NONE and nothing else, mirroring the software pools, which already reject anything that is not one of their three formats. A pool that declines the format never becomes configured, its renderer is discarded, and it is never given a visible renderer -- so the two sets of pools cannot steal each other's streams regardless of registration order. IsCompatible() was returning true unconditionally; it now checks the scaling method like every other pool, and CRPRendererFBO::SupportsScalingMethod gains SCALINGMETHOD::AUTO to match CRPRendererOpenGL. Without AUTO -- the default -- the pool failed IsCompatible on every frame, and because GetRendererForSettings consults no cache before iterating pools, that logged once per frame indefinitely. The FBO factory is registered last so the search, which stops at the first match, settles on DMA for software streams without consulting it at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CGameClientStreams::EnableHardwareRendering() returned false unconditionally with a "not implemented" error, before any stream was created. That was the first thing a hardware-rendering core hit, so nothing further down the path was reachable. It now accepts OpenGL and OpenGL ES contexts and rejects anything else -- Vulkan, in practice -- while the core can still fall back to software, rather than letting it fail later inside context_reset(). CRetroPlayerRendering::OpenStream() hardcoded 640x480 and then returned false. game_stream_hw_framebuffer_properties carries no geometry, so the frame size is genuinely unknown at open time; configuration is now deferred to the first GetStreamBuffer() call, which is handed the size the core wants. CloseStream(), previously empty, releases the shared context. GetStreamBuffer() no longer reports success while handing back a framebuffer of 0, which would have sent the core's drawing to the default framebuffer. CRPRenderManager::Create() and RenderFrame() were both stubs. RenderFrame() promotes the buffer the core just rendered into so the rendering thread samples it, mirroring what AddFrame() does for software frames, and marks it loaded since there is nothing to upload. GetCurrentFramebuffer() now releases the previous frame's buffer instead of leaking it, and refuses to hand out a framebuffer of 0. CGameClientStreamHwFramebuffer::GetBuffer() passed 0,0 rather than the requested size, so lazy allocation could never work. CRenderBufferPoolFBO::DestroyContext() is broadcast to every pool, so it now returns early rather than destroying a context it never created. A core still produces no frame: HardwareContextReset() is invoked on whatever thread opened the stream, before any context is current and before the framebuffer exists, so a core that builds its GL objects in context_reset() fails there. That ordering is the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the hardware rendering path. A libretro core that negotiates
OpenGL now renders into a framebuffer Kodi owns, and Kodi samples the
texture behind it.
Context lifecycle
-----------------
The shared context is created and made current before the client is told
the context is ready, on the game loop thread -- the thread the client will
run on, and the only one a context may be current on. CRetroPlayerRendering
drives this via a new IRenderBufferPool::CreateContext(), which mirrors the
existing DestroyContext(). Teardown deletes the framebuffer while its
context is still current and destroys the context on the same thread that
made it current; the pool destructor no longer attempts either, as it runs
on whichever thread drops the last reference.
The context requested is now the one the client asked for. It was hardcoded
to GL 3.2 core profile, so a core written against legacy OpenGL -- which has
no core-profile equivalent for what it uses -- could not build its resources,
with no symptom beyond a core that never draws.
Ordering
--------
Clients ask for their framebuffer from inside context_reset, so:
- The framebuffer is allocated before the client is notified, not lazily on
first request. game_stream_hw_framebuffer_properties gains the frame size
to make that possible; it is carried on the stream rather than alongside
the other hardware properties because it comes from the client's system
AV info, which is not available when hardware rendering is negotiated.
- HardwareContextReset() is no longer invoked while the stream is being
opened. The stream handle does not exist until the open call returns, so
the client's request had nowhere to go. The add-on now resets the context
once its stream is open.
- GetCurrentFramebuffer() no longer waits for a visible renderer. Rendering
into a framebuffer nothing samples yet costs a frame; returning 0 sends
the client's drawing to the default framebuffer for the rest of the
session, because clients keep the framebuffer they were given.
The framebuffer is held for the life of the stream rather than being looked
up per frame, since the client renders into the same one throughout.
Quieting
--------
A buffer pool that declines the stream's pixel format will decline it every
frame, and configuration is attempted per pool per frame. Pools now remember
the refusal, and no renderer is built for a pool that cannot serve the
format, which keeps the software pools silent during a hardware stream.
Also wires HwContextDestroy, which the Game API has always carried and Kodi
has never called.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d for The framebuffer was a bare glGenFramebuffers with a colour texture. Every property the client sent was captured, logged and then ignored. Depth and stencil are now attached as a renderbuffer sized to the frame: a packed 24/8 buffer when both were asked for, depth alone otherwise, and stencil without depth ignored as the Game API specifies. Nothing samples them afterwards, so a renderbuffer rather than a texture is enough. craft asks for depth, and without it its 3D geometry sorted by draw order. bottom_left_origin is honoured. Clients using top-left origin semantics have written the image the other way up relative to how the texture is sampled, so the V coordinates are swapped for them and left alone for everyone else. The EGL config no longer asks for a window surface or a depth buffer. This context is only ever made current without a surface, and depth now lives in an attachment built to the client's request, so both constraints only narrowed the matching configs. Context version and profile are validated by asking the driver for what was requested and letting it refuse, rather than testing against a hardcoded table that would go stale. A refusal now names the context that could not be provided and fails the stream cleanly, leaving the client free to fall back to software rendering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The game client renders on its own context and thread while the rendering thread samples the texture that context is drawing into. The two submit work independently, so nothing stopped the GPU sampling the texture before the client's commands had run. That shows up as tearing or a half-drawn frame, and is the kind of fault that looks fine on one driver and not another. A fence is recorded on the client's context once it reports the frame complete, and the rendering context waits on that fence before sampling. The wait is server-side, so it orders the GPU's work without stalling the rendering thread, which is what keeps the game loop independent of vsync. The fence is flushed when recorded, as a fence sitting unsubmitted in the client's command buffer can never become signalled for the other context. SetFence and WaitFence are declared on IRenderBuffer with no-op defaults, rather than the render manager reaching for a GL type it cannot name on every platform. Note this orders the client's writes ahead of our reads, but not our reads ahead of the client's next frame: with a single framebuffer the client can begin overwriting the texture while it is still being sampled. Whether that is visible in practice decides if a second buffer is worth the memory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gameplay stuttered badly while Kodi's own frame rate looked healthy, which is the signature of the game loop being throttled rather than the renderer struggling. The client rendered into a single texture that the rendering thread sampled from. Drivers insert implicit synchronisation when one context writes a texture another is reading, so the client's next frame could not start until our sampling had finished, pacing the game loop to the display's refresh instead of letting it run freely. The client is now handed each of two buffers in turn, so the frame being drawn is never the frame being sampled. This also closes the hazard left open by the fence work: the fence orders the client's writes ahead of our reads, and alternating buffers keeps our reads ahead of the client's next frame. Each buffer also owns its framebuffer now. Previously the pool held one framebuffer whose attachments were rebuilt and revalidated on every single frame, glCheckFramebufferStatus included, because the client re-acquires its framebuffer each frame. Attachments are made and validated once at allocation, and the per-frame path is now just returning the ID. Owning a framebuffer per buffer is also what lets several be in flight without fighting over one set of attachments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stopping a game left Kodi unresponsive for around half a minute before the GUI came back. The game loop tears the rendering context down as it ends, through IGameLoopCallback::EndEvent. The stream was closed afterwards, from whichever thread stopped the player, and released the framebuffers and textures there -- by which point the context that owned them was gone, so the deletes ran with no context current. The buffers are now released as part of destroying the context, on the thread the context is current on. Closing the stream still asks, as a backstop for a stream closed without the game loop having run, and finds nothing to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… at open Kodi aborted with "eglSwapBuffers failed (EGL_BAD_SURFACE)" whenever a hardware-rendering client opened its stream from Kodi's own rendering thread. mupen64plus-nx does exactly that: it opens its video stream while the game is loading, before the game loop thread exists. The context was made current when it was created, on whichever thread happened to open the stream, and left that way. An EGL binding is per-thread, so when that thread was Kodi's own, Kodi lost the surface it presents with. The earlier assumption that the opening thread is always the game loop holds for clients that open their stream from inside a frame, and not otherwise. The context is now bound around each call into the client and released afterwards, on whichever thread makes the call, restoring exactly what that thread had. Every entry point is covered -- load, standalone load, reset, unload, run frame, and both context callbacks -- because a client may make rendering calls anywhere inside any of them, not just between asking for a framebuffer and presenting one. Binds nest, since allocating buffers brackets the context too and that happens inside a frame for some clients. Two things this makes safe. Framebuffer objects are not shared between contexts, so the pool now refuses to build a buffer unless the client's context is current, which also stops the rendering thread quietly allocating one in Kodi's context. And a context can only be current on one thread, so a second thread asking for it is refused and logged rather than left to fail somewhere less obvious. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two faults from the buffering work, both found by playing rather than by the acceptance run. Content from earlier frames bled into later ones, most visibly a title screen persisting into the menu after it. Clients treat the framebuffer they are given as theirs for the session and many do not clear it, drawing over what the last frame left behind. Handing them alternate buffers gives each only every other frame's drawing, so the frames in between show through. Back to one. Gameplay also ran far below the rate the renderer reported, which is the signature of the game loop being paced rather than the renderer struggling. With a single texture the server-side wait before sampling serialises the two: the client cannot start its next frame until we have finished with this one. Removing it restores full speed. Ordering now rests on flushing the client's commands when its frame ends and the implicit synchronisation drivers apply to a texture shared between contexts, which is what other libretro frontends do with a single framebuffer. The renderer also left the client's texture bound in Kodi's context, which is a dangling binding for the GUI to draw with once the client's context and its textures are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…text A hardware-rendering client's teardown ran after its context had already gone. The game loop destroys the context as it ends, and the stream is closed afterwards from whichever thread stopped the player, so the client's context_destroy executed against Kodi's context instead of its own. A client releasing its resources there deletes Kodi's objects by name. BeginClientFrame now reports whether the context is actually current, and the client is only asked to release its resources when it is. Once the context is gone those resources went with it, so there is nothing left to release. Also brackets the FBO renderer's drawing with the render context's state block, the same way Kodi brackets a visualisation or screensaver add-on. Kodi's drawing depends on state it sets up once and expects to stay put, the global vertex array object above all: without it bound, every GUI draw is silently discarded and the screen goes black. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fence had no consumer. Waiting on it serialised the client against the renderer and cost most of the frame rate, so the wait was removed, which left a sync object created and destroyed every frame for nothing, and two virtuals on IRenderBuffer that every buffer type had to carry. Ordering comes from flushing the client's commands at the end of its frame. The framebuffer was held in a vector with a write index and a modulo step, which is machinery for a rotation that cannot happen: a client keeps the framebuffer it is given and draws over what the last frame left, so it can only ever have one. A single pointer says that, and the comment explains why it will stay one. Also drops a note describing these functions as examples pulled from the history of the OpenGL effort. They are implementations now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A game client that renders on the GPU shares it with Kodi and does not put back what it changes. The vertex array object Kodi binds once and expects to stay bound is the one that matters: while it is unbound every GUI draw is rejected, so stopping a hardware-rendered game left the screen black with a perfectly responsive Kodi behind it. Re-apply Kodi's state block once the client has unloaded, which is the same thing Kodi does after a visualisation or screensaver add-on has had the context. It has to be after the unload, not at stream close: the client releases its GPU resources as it unloads, and those calls land after anything done earlier. Savestates need the same care from the other side. They are built out of frame pixels, which a hardware-rendered frame does not have -- it lives in a GPU framebuffer and is never read back. Worse, there is only one such buffer, and the thumbnail path acquired it and returned on the error without releasing it, so every autosave permanently pinned a reference to the framebuffer the client was drawing the next frame into. Skip the frame work for hardware rendering, and release the buffer on the way out of that error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A hardware-rendering client's GPU resources are only reachable while its context is current, and the calls that reach for them are not just the ones that draw. A client that serializes its video state answers SerializeSize(), Serialize() and Deserialize() out of GPU memory, and one that builds its renderer off the back of its reported geometry does that work inside GetGameTiming(). All of them ran outside the guard. Found with Dolphin, which serializes its whole machine -- framebuffer manager included -- when asked how large a savestate is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A client serializes the machine it emulates, and some clients build that machine while booting the game, so there is nothing to measure until a frame has run. Kodi asked while loading, before any frame -- Dolphin walks its video and hardware state to answer, and crashed on neither existing yet. Nothing needed the answer that early. Rewind and savestates cannot happen before a frame either, and every caller already reads it live and handles a size of zero, so ask on first use instead. The size is cached once a client has answered, and forgotten when the file closes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…elsewhere The FBO renderer and its buffer pool were built only for GBM and Wayland, and registered only there, so an X11 session got no hardware rendering at all -- no Dreamcast, no hardware N64. Neither actually depends on those windowing systems; they need desktop GL and a shared EGL context, so build them on that basis and register the renderer on X11 too. X11's process info is now the EGL one, which is what lets a client resolve GL functions there. X11 is the reason the second half matters. It runs on EGL by default but falls back to GLX, where there is no context to share and hardware rendering cannot work. A client asks whether it can render this way long before the frontend would try to build it a context, and until now the answer was always yes: the context then failed at stream-open, and the client -- already wired up to callbacks that were never installed -- ran a frame and jumped through a null pointer. Ask the process info while the client is still negotiating, so it is told no while it can still fall back to software. Its own EGL accessors are what X11 answers with, so CWinSystemEGL's are now virtual; a dynamic_cast to it previously found the base's empty context. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A client that renders on the GPU asks for a particular OpenGL version, and some ask for a lot: Kronos wants 4.3 with compute shaders. Until now a system that could not provide it got a generic "OpenGL support is still under development", which says nothing about why the game will not start and has not been true for a while. Compare what the client asks for against what the render system reports, and where it cannot be met, say so with both versions in the message. The same applies when the display stack cannot do hardware rendering at all, which is a different sentence because there is no version to compare against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The savestate folder for a game is named after the game's file, and Kodi cannot always create it -- a name carrying brackets is enough. MakePath() gives back an empty string when that happens, and listing an empty path enumerates the root of the filesystem, so the dialog asking which savestate to load offered /bin, /dev and /root, and picking any of them failed. It also meant a game whose savestate folder could not be made was unplayable: the dialog is shown before a game client is chosen, and cancelling it aborts playback. Treat the empty path as the failure it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…w into A hardware-rendering client is given a framebuffer sized for the largest frame it said it would ever draw, and most frames are smaller than that. Nothing told the frontend how much of it had been drawn, so the whole framebuffer was put on screen: the image sat in a corner of a larger black field, at the wrong shape, and stretching it in the OSD only stretched the black with it. Carry the size of each frame in the packet the client already sends when it has finished drawing. The buffer then reports the frame it holds rather than its allocation, which is what the renderer measures the image's shape and position from, and texture coordinates are taken against the texture instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clients ask for their framebuffer once and render against it for the rest of the session, so when one draws nothing the first question is what it was handed. Answering it previously meant a rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ossible The check asked the window system for an EGL display, which is the wrong question. A build without desktop OpenGL has no FBO pool compiled in at all, yet still has an EGL display, so it would tell a client hardware rendering was available and then have nothing to render with -- the same failure this check was added to prevent, on a different platform. Ask the pools instead. Only a pool that owns a rendering context answers yes, and a build without one has none, which is true everywhere without needing to know anything about the display stack. The limitation is documented where the pool is declared: OpenGL ES clients are refused during negotiation and fall back to software. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A buffer pool that shares its memory with the GPU saves copying every frame, and is what a game is drawn through by default where the display stack allows it. The two ends have to be kept in step, though, and where they are not the picture comes out with rows of speckled rubbish through it -- visible in any software-rendered game. Keeping them in step properly is the real fix and is not this. Until then, offer the way past it: drawing through OpenGL instead is unaffected, and here is no slower. Off by default, so nothing changes for anyone not seeing the problem. The help text describes what a player sees rather than how any of this works, since neither name means anything to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pool is built only where desktop OpenGL is present, and a build is either desktop GL or GLES, never both, so the ES side of these was unreachable. It also read as though OpenGL ES were supported here, which it is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
|
Garbear - aware you've got a lot on - this is super exciting functionality i think you'd be interested in. I'm wanting lots of feedback from a wide base of users on code structure and all the errors it probably contains. Ive added as much defensive programming as i can but aware i've not consulted anyone really on what is quite a complicated architecture. Also wanting to open this up to the libreelec team for review too. Happy to take any feedback on approach curious for you to test but it genuinely works and gets us a long way to full compatibility. Edit: I'll also eventually squash commits keeping them all visible at the moment in case everyone seems happy and we can look at incremental cherry pick merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paired with kodi-game/game.libretro#164, which must land alongside this one — the Game API version moves to 7.0.0. How strictly Kodi should refuse older add-ons is an open question with a section of its own below.
Marked experimental: it changes the Game API, and parts of it are unverified on hardware I do not have — not because it is fragile in use.
Builds on Lukas Rusak's WIP FBO buffer/pool/renderer commits, which are preserved
with their original authorship at the base of this branch.
What this is
Game clients that render with OpenGL — Flycast, Mupen64Plus-Next, melonDS,
PPSSPP, YabaSanshiro and others — could not run in Kodi at all. RetroPlayer only
accepted frames a client had drawn in software and handed over as pixels. This
adds the path that lets a client render on the GPU instead: Kodi gives it a
framebuffer, the client draws into it, and Kodi draws the texture that
framebuffer is backed by. The frame never leaves the GPU.
Six systems run well on it today, including several that Kodi has never been
able to play at all. It is marked experimental because it changes the Game
API and because parts of it are unverified on hardware I do not have, not
because it is fragile in use — every core listed below plays and quits cleanly,
repeatedly.
What works
Verified by running real games to a picture on screen, then quitting back to a
working GUI:
Also working: rewind and savestates alongside hardware rendering; internal
resolution upscaling (an N64 game at 1920x1440 allocates and uses the larger
framebuffer); quitting a game returns to a working GUI, which was the single
hardest problem in this branch.
Kronos (Saturn/ST-V) negotiates an OpenGL 4.3 core profile with packed
depth/stencil — the most demanding request of any core tested, and served
correctly — but renders black for reasons inside the core, not the frontend.
Screenshots
All taken in Kodi with the debug overlay left on, so the frame rate and the fact
that this is Kodi are both visible. Four different systems, each rendering on the
GPU through this branch.
Dreamcast — Flycast, Airforce Delta, 60.5 fps
PlayStation Portable — PPSSPP, Gran Turismo, 59.5 fps
Nintendo DS — melonDS, Ridge Racer DS, 58.5 fps — both screens
Nintendo 64 — Mupen64Plus-Next, Mario Kart 64, 59.6 fps
The feature is gated, and fails politely
Nothing here is reachable unless the build and the hardware can actually serve
it, and a client is told during negotiation rather than part-way through:
that hardware rendering is unavailable, and the client falls back to software.
GLX, does the same.
a dialog naming both versions: "This game renders with OpenGL 4.3, but this
system only provides OpenGL 3.1. It can't be played on this hardware."
This matters because the failure it replaces was not graceful. A client told
"yes" wires itself up to callbacks it will call regardless, and then jumps
through a null pointer when they were never installed. Refusing early is what
turns a crash into a sentence the user can act on.
What this does not do
No OpenGL ES. The buffer pool, buffer and renderer are built only where
desktop GL and EGL are both present. A GLES build has no pool that offers
hardware rendering, so clients are told during negotiation and fall back to
software. This excludes Android and ARM builds, and is the largest gap.
X11 is wired up but unverified. The refusal path was tested (X11 falling
back to GLX correctly declines and the client falls back to software). The
working path — X11 on EGL — has never been run, because the test machine has
no X11 session available.
No GameCube. Dolphin gets a long way — it loads a disc, boots into
GameCube mode, brings up its OpenGL backend on the context we give it, and
runs frames — then fails inside its own renderer. Four separate problems were
found and worked through:
including the video backend. Kodi asked during load, before a frame had
run, so it crashed in
FramebufferManager::DoStateon a renderer that didnot exist yet. Fixed here, by asking on first use instead.
SerialInterface::ISIDevice::GetDeviceTypeinstead: its serial interface devices are built during the emulated boot
too, so the same question was still too early.
buttonmap.xmlortopology.xml, so everycontroller port reported
RETRO_DEVICE_NONEand no controller was attachedat all. Its install also drops 4 of its 48
Sys/Shadersfiles.VertexManagerBase::Flush, preceded byGL_INVALID_FRAMEBUFFER_OPERATION in glClear(incomplete framebuffer).The frontend was ruled out for this one: instrumentation confirmed we never
hand it framebuffer 0, and the FBO it is given allocates cleanly.
The first is a genuine frontend bug this branch fixes and every core benefits
from. The rest are in Dolphin's libretro port and its add-on packaging.
No unit tests. RetroPlayer has none today, and adding the first set inside
a feature branch felt like the wrong place to start that conversation. Happy
to — the pure logic worth covering is noted below.
No measured performance numbers. The improvement over software rendering
is plain in use, but no clean measurement was taken on an idle machine, so
none is claimed here.
The API change, and how much of one it really is
ADDON_INSTANCE_VERSION_GAMEmoves to 7.0.0. Two structures gained fields:game_stream_hw_framebuffer_properties— the largest frame the client willdraw, so the framebuffer can be sized before the client asks for it. It
previously held only a placeholder byte. 1 → 8 bytes.
game_stream_hw_framebuffer_packet— the size of each frame actually drawn.Without it the whole framebuffer is displayed, so the picture sits in a corner
of a larger black field at the wrong shape. 8 → 16 bytes.
Neither growth is visible across the ABI boundary. Both live inside unions
whose largest members are the software-rendering structures, and both stayed
below them. Measured on x86-64 with the 6.0.0 header and this one, the structures
that actually cross the boundary are byte-identical:
game_stream_propertiesgame_stream_packetThe new fields are also unreachable for an older add-on. Kodi reads them only
after a client has opened a
GAME_STREAM_HW_FRAMEBUFFERstream, and no 6.0.0add-on can:
EnableHardwareRendering()returned false unconditionally and theproperties buffer was uninitialised, so the type read as
GAME_STREAM_UNKNOWNand the stream was refused. Hardware rendering has never once worked end to end,
which is why nothing depends on its old layout.
So
_MINis a choice rather than a consequence, and it is the one thing hereworth deciding rather than assuming:
_MINat 6.0.0 lets existing add-ons keep loading and running insoftware, and rebuilt ones gain hardware rendering — rollout becomes gradual.
_MINto 7.0.0 (what the branch does today) makes Kodi refuse everyadd-on until it is rebuilt. Safe by construction, no reasoning required, but a
flag day for all binary game add-ons.
The branch sets
_MINto 7.0.0 because that is the conservative default and itis a one-line change either way. Worth noting that the achievements work already
in front of this adds three entries to the function table, which is an
unavoidable break — if both land in the same cycle, the rebuild is happening
regardless and there is no second cost to the strict choice. If they land apart,
holding
_MINat 6.0.0 avoids a second flag day. Happy to go whichever waykodi-game prefers.
Notable fixes along the way
Each of these was a real defect, found by running cores rather than by reading:
GPU with Kodi and does not restore what it changes; the vertex array object
Kodi binds once and relies on is the one that matters. Kodi's state block is
re-applied once the client has unloaded — it must be after the unload, since
the client releases its GPU resources as it goes.
Clients that build the machine they serialize during the game's boot cannot
answer. It is now asked for on first use.
GPU resources —
SerializeSize,Serialize,DeserializeandGetGameTiming.frame does not have, and the thumbnail path leaked a reference to the client's
only framebuffer on every autosave.
Testing
Six systems as above, on GBM and Wayland, Mesa 26.0.3 on Intel Arc. Software
cores re-checked throughout to confirm they still take the software path. Every
run verified that the GUI came back after quitting, not merely that Kodi
survived.
Not tested: OpenGL ES, X11 on EGL, Windows, macOS, Android, or any non-Intel GPU.
Fixes this turned up outside Kodi
Getting six systems running meant fixing things in the cores and their add-ons
as well. These are all submitted, and none of them are needed for this branch to
be reviewed — they are listed because they are the reason the cores in the table
above behave:
hrydgard/ppssppglewInit()fails on Wayland and GBM. AddedGLEW_EGL=1.kodi-game/game.libretro.flycastlibretro/melonDS-lGLunconditionally, so it cannot build for a GLES-only system.FCare/KronosLibreELEC/LibreELEC.tvTwo parts that can be split out on request
Both are small, independent, and carried here only because this branch is where
they were found. Say the word and either can become its own pull request.
The savestate dialog listing the filesystem root. When Kodi cannot create a
game's savestate folder — which happens for any filename containing brackets,
so most No-Intro and Redump names — it was left with an empty path, and listing
an empty path enumerates
/. The dialog then offered/bin,/devand/rootas savestates, and since it is shown before a game client is chosen, cancelling
it aborted playback. That made such games unplayable, on any renderer, with or
without this branch. Only the dangerous half is fixed here; the folder still
fails to be created, which belongs with the savestate work.
Separable whenever wanted — it touches only
CSavestateDatabaseand nothingelse here depends on it.
The "Enable OpenGL" game setting. A buffer pool that shares its memory with
the GPU avoids copying every frame and is the default where the display stack
allows it, but the two ends are not kept in step, and rows of speckled rubbish
come through in any software-rendered game. This adds a setting to draw through
OpenGL instead, off by default. It is a way past the problem, not a fix for it.
Separable once someone owns the underlying synchronisation fix — until then it
is the only way for an affected user to get a clean picture, which is why it is
here rather than waiting.
Notes for review
IRenderBufferPool::SupportsHardwareRendering()is what decides whether abuild can offer this at all, and is the single place to look when asking "what
happens on platform X".
ConfigureInternal()returningm_format == AV_PIX_FMT_NONE— is the only thing keeping software cores offthis path and hardware cores on it. It is load-bearing and would be the first
thing worth a test.