Skip to content

vrend: fix sampler view/state dirty tracking and texture bindings

Ryan Neph requested to merge ryanneph/virglrenderer:rn-fix-dirty-tracking into main

There is a fundamental issue with the current sampler view/state dirty tracking that vrend uses to elide unnecessary calls to glBindSampler()/glBindTexture() during pre-draw setup.

I noticed it while investigating the PDF rendering artifacts described in #526 (closed). In that case, I found that poisoning the dirty mask with all bits set at all times, to essentially disable the dirty state tracking and always emit the driver state updates before each drawcall was sufficient to fix the issue.

In a deeper investigation I found that we have the following problematic sequence of API calls:

  1. bind a new shader (glUseProgram()) and set all samplers to "dirty"
  2. during draw-call preparation, in vrend_draw_bind_samplers_shader(), emit a sequence of glActiveTexture() + glBindTexture() + glBindSampler()
  3. unset sampler "dirty" bits to elide state changes before the next draw-call for the same shader and sampler sets
  4. Some unrelated texture state update that does one or more glBindTexture() on the last-used glActiveTexture()
  5. next draw-call elides the sampler setup since no dirty bits set

Essentially step (4) invalidates our assumption that the texture to sampler binding state remains invariant, and we actually would need to emit those state updates again.

The solution I've arrived at to the above, while retaining the use of dirty state tracking for its performance benefits is to keep the glActiveTexture() always set to some texture unit that we almost-certainly would never use (GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS - 1).

Perhaps we can do something smarter, but this is already an improvement on the current broken assumption of sampler state invariance that lead to #526 (closed).

Most commits in the MR are refactors, with the bugfix coming in the last commit.

Fixes: #526 (closed)

/cc @gerddie @tintou

Edited by Ryan Neph

Merge request reports

Loading