XFB: The Re-Return of the Sequel I Didn’t Want
In the course of handling ARB_enhanced_layouts, I came across another interesting xfb-related issue: shaders are now allowed to specify all the xfb buffer info in the layout, and they’re also now allowed to set the location of the variable. And locations aren’t considered to overlap unless they have overlapping components.
For those who recall so many years ago when I explored xfb integration in ntv, you know that this is a problem because I’ve been tracking xfb outputs based solely on the location.
That is to say:
SpvId var_id = spirv_builder_emit_var(&ctx->builder, pointer_type, SpvStorageClassOutput); ctx->outputs[var->data.location] = var_id;
It looks a bit like this, where the variable id (and type info) is indexed using nir_variable::data.location, with the assumption that no variables will share the same location.
Yeah, in retrospect, this was stupid. I already had some parts of the code here which accounted for collision of base locations (e.g., one component from a variable might be output to buffer A, and another component might be output to buffer B), but I didn’t account for it here.
For the “good” handling, I’ve been using a key value of
nir_variable::data.location << 2 | nir_variable::data.location_frac. This expands the base location to a 4-component index (because slots are sized for
vec4 types), and then adds in the component usage to give a precise index that has no conflicts.
In some cases, there are output variables which don’t have component (
location_frac) set, but then the xfb output uses the component value. Luckily, this is a simple case to handle: using the expanded index from above, the base index can be found with a simple loop from the varying slot location of the xfb info.
SpvId output = ctx->outputs[location]; while (!output) output = ctx->outputs[location--];
At most, this will go back three indices before it finds the output, which will always be nonzero.
I’m optimistically hoping that this will be the end of xfb handling, but probably it won’t be.