Skip to content

Fix: Clear stale Skyboxify sky layers to avoid frametime spikes (#108) - #109

Open
gnilobaiter wants to merge 1 commit into
Zolo101:stonecutterfrom
gnilobaiter:fix/skyboxify-stale-sky-layers
Open

gnilobaiter wants to merge 1 commit into
Zolo101:stonecutterfrom
gnilobaiter:fix/skyboxify-stale-sky-layers

Conversation

@gnilobaiter

Copy link
Copy Markdown

Clear stale Skyboxify sky layers (optional compat fix)

Refs #108, related to #90.

This is an optional compat workaround. Feel free to drop it. If it looks useful, it could go into a future release as a fix for a mod whose source is closed, so it can't be fixed upstream.

The problem

With RenderScale + Skyboxify + a custom sky pack (Dramatic Skys), there is a large frametime spike (~200-600 ms in my logs) when leaving water after ~10-20 seconds underwater. The spike gets worse as the render scale goes up (none at 100%, small at 105%, bigger at 150%, worst at 200%).

Cause

Skyboxify adds its sky layers to an internal queue (FeatureRenderer.submits) every frame during extraction, but only clears it in endFrame, which runs when the vanilla sky pass renders. Vanilla skips the sky pass in some situations (it happens underwater), so the queue is never cleared and grows by a few layers per frame. When the sky pass runs again, every queued layer is drawn in one frame.

I logged the queue size while underwater: it grew linearly, 180 -> 7260 in ~6 seconds, and dropped back to 4 as soon as I surfaced. In that same frame Dynamic Transforms UBO resized from 32 up to 2048+ (16384 and 32768 after longer dives) and the frame took 200+ ms. All of these are full-screen layers, so the cost scales with the render target size, which is why it shows up with RenderScale.

What this changes

  • New SkyboxifyCompat: after renderLevel finishes, it calls the public FeatureRenderer.clear() on Skyboxify's queue.
  • MixinGameRenderer.handBack calls it once per frame.

In a normal frame the queue is already empty after endFrame, so this is a no-op. When the sky pass was skipped, it drops the layers that would otherwise pile up.

Implementation notes

  • Everything is reflective, so Skyboxify stays an optional dependency. If the classes/fields are missing (Skyboxify not installed, or a different layout) it silently does nothing. If a reflective call throws, it logs a warning once and disables itself.
  • It logs one info line when Skyboxify is detected.
  • The proper fix belongs in Skyboxify: clear the queue even when the sky pass is skipped. This just works around it on our side.
  • Field/class names are taken from Skyboxify 3.4 for 26.3 (skyboxify$skyFeatureRenderer on SkyRenderer, FeatureRenderer#clear).

Testing

  • Built 26.3-fabric only. I have not built the other version targets, but the change is pure reflection plus one call, so it shouldn't affect them.
  • In game on 26.3 Fabric (Skyboxify 3.4, Sodium, Dramatic Skys, scale 200%): the spike on leaving water is gone, and Dynamic Transforms UBO no longer grows after diving.
  • Smaller frametime spikes while chunks load still happen. They don't look related to this change or to Skyboxify.

…101#108)

Skyboxify queues its sky layers every frame and only clears the queue
when the vanilla sky pass renders. When the sky pass is skipped (e.g.
underwater) the queue grows by a few layers per frame and is drawn all
at once when the sky returns. With custom sky packs and a high render
scale this shows up as a large frametime spike when surfacing.

Clear the queue after the level has rendered via a reflective, optional
compat hook. It is a no-op when Skyboxify is absent or the queue is
already empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant