Skip to content

nv2a: Implement texture LOD bias #2435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 23, 2025
Merged

nv2a: Implement texture LOD bias #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b8ada79
nv2a: Implement texture LOD bias
abaire Sep 18, 2025
9c9f43c
SQUASHME: CR updates
abaire Sep 19, 2025
ebbb520
SQUASHME: Renamed convert_lod_bias for namespacing purposes and clarity
abaire Sep 19, 2025
ec5ed5e
SQUASHME: Clamp negative LOD bias as well
abaire Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hw/xbox/nv2a/pgraph/gl/renderer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ typedef struct TextureBinding {
unsigned int scale;
unsigned int min_filter;
unsigned int mag_filter;
uint32_t lod_bias;
unsigned int addru;
unsigned int addrv;
unsigned int addrp;
Expand Down
8 changes: 8 additions & 0 deletions hw/xbox/nv2a/pgraph/gl/texture.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ static void apply_texture_parameters(TextureBinding *binding,
{
unsigned int min_filter = GET_MASK(filter, NV_PGRAPH_TEXFILTER0_MIN);
unsigned int mag_filter = GET_MASK(filter, NV_PGRAPH_TEXFILTER0_MAG);
unsigned int lod_bias =
GET_MASK(filter, NV_PGRAPH_TEXFILTER0_MIPMAP_LOD_BIAS);
unsigned int addru = GET_MASK(address, NV_PGRAPH_TEXADDRESS0_ADDRU);
unsigned int addrv = GET_MASK(address, NV_PGRAPH_TEXADDRESS0_ADDRV);
unsigned int addrp = GET_MASK(address, NV_PGRAPH_TEXADDRESS0_ADDRP);
Expand Down Expand Up @@ -146,6 +148,11 @@ static void apply_texture_parameters(TextureBinding *binding,
pgraph_texture_mag_filter_gl_map[mag_filter]);
binding->mag_filter = mag_filter;
}
if (lod_bias != binding->lod_bias) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

binding->lod_bias not initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, done.

binding->lod_bias = lod_bias;
glTexParameterf(binding->gl_target, GL_TEXTURE_LOD_BIAS,
pgraph_convert_lod_bias_to_float(lod_bias));
}

/* Texture wrapping */
assert(addru < ARRAY_SIZE(pgraph_texture_addr_gl_map));
Expand Down Expand Up @@ -727,6 +734,7 @@ static TextureBinding* generate_texture(const TextureShape s,
ret->data_hash = 0;
ret->min_filter = 0xFFFFFFFF;
ret->mag_filter = 0xFFFFFFFF;
ret->lod_bias = 0xFFFFFFFF;
ret->addru = 0xFFFFFFFF;
ret->addrv = 0xFFFFFFFF;
ret->addrp = 0xFFFFFFFF;
Expand Down
9 changes: 9 additions & 0 deletions hw/xbox/nv2a/pgraph/texture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,13 @@ hwaddr pgraph_get_texture_palette_phys_addr_length(PGRAPHState *pg, int texture_
TextureShape pgraph_get_texture_shape(PGRAPHState *pg, int texture_idx);
size_t pgraph_get_texture_length(PGRAPHState *pg, TextureShape *shape);

static inline float pgraph_convert_lod_bias_to_float(uint32_t lod_bias)
{
int sign_extended_bias = lod_bias;
if (lod_bias & (1 << 12)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this 12th bit have any significant name or is it merely tested to "or in" the subsequent 0x00001FFF const? And whats the significance of that? Fixed point ceiling, because of the final division by 256.0f ? Xemu is not strong in code comments but this looks like a hard to maintain bit of logic of anyone ever needs to revisit this again.

Copy link

@PatrickvL PatrickvL Sep 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, it is actually doing sign extension on the 12th bit onwards, assuming the trust contains a 12 bit signed fixed point number (4 integer + 8 fractional bits, in twos complement, a.k.a. Q3.8). Apparently there's no helper function for that yet.

Just 1 thought: the output range is now -8.0 to +7.99609375 which is not exactly 8.0
Would that be any problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there are many places where 5.8 fixed point numbers are used and it doesn't seem clearly useful to bother with a more complicated generic sign extended fixed point -> float conversion function.

I also doubt it'd 7.99609375 versus 8 would result in noticeable differences if it actually ends up being materially different from how the nv2a HW handles the 5.8 value. I'd guess there's likely more variance across HW/drivers than that fractional delta in any case.

sign_extended_bias |= ~NV_PGRAPH_TEXFILTER0_MIPMAP_LOD_BIAS;
}
return (float)sign_extended_bias / 256.f;
}

#endif
10 changes: 9 additions & 1 deletion hw/xbox/nv2a/pgraph/vk/texture.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,14 @@ static void create_texture(PGRAPHState *pg, int texture_idx)
min_filter == NV_PGRAPH_TEXFILTER0_MIN_BOX_NEARESTLOD ||
min_filter == NV_PGRAPH_TEXFILTER0_MIN_TENT_NEARESTLOD;

float lod_bias = pgraph_convert_lod_bias_to_float(
GET_MASK(filter, NV_PGRAPH_TEXFILTER0_MIPMAP_LOD_BIAS));
if (lod_bias > r->device_props.limits.maxSamplerLodBias) {
lod_bias = r->device_props.limits.maxSamplerLodBias;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to also handle negative bias clamp (absolute value must not exceed maxSamplerLodBias)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, misread your comment, sorry!

That said, is it actually necessary to manually clamp these at the application level?

The docs lead me to believe that the driver would handle the clamping, since it ends up being a summation of the bias and the sampling so simply clamping the bias may be insufficient to remain in bounds anyway:

maxSamplerLodBias is the maximum absolute sampler LOD bias. The sum of the mipLodBias member of the [VkSamplerCreateInfo](https://registry.khronos.org/vulkan/specs/latest/man/html/VkSamplerCreateInfo.html) structure and the Bias operand of image sampling operations in shader modules (or 0 if no Bias operand is provided to an image sampling operation) are clamped to the range [-maxSamplerLodBias,+maxSamplerLodBias]. See https://registry.khronos.org/vulkan/specs/latest/html/vkspec.html#samplers-mipLodBias.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it actually necessary

Maybe, maybe not. According to spec: The absolute value of mipLodBias must be less than or equal to VkPhysicalDeviceLimits::maxSamplerLodBias

the driver would handle the clamping

In general, Vulkan drivers are a lot less forgiving. Even when operating fully within spec, they can be problematic. If we are lucky, a case like this would trigger a validation warning and have no other impact, if we are unlucky it results in some random overflow and a crash, so it's best to follow spec guidance when populating struct params.

} else if (lod_bias < -r->device_props.limits.maxSamplerLodBias) {
lod_bias = -r->device_props.limits.maxSamplerLodBias;
}

VkSamplerCreateInfo sampler_create_info = {
.sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO,
.magFilter = vk_mag_filter,
Expand All @@ -1356,7 +1364,7 @@ static void create_texture(PGRAPHState *pg, int texture_idx)
VK_SAMPLER_MIPMAP_MODE_LINEAR,
.minLod = mipmap_en ? MIN(state.min_mipmap_level, state.levels - 1) : 0.0,
.maxLod = mipmap_en ? MIN(state.max_mipmap_level, state.levels - 1) : 0.0,
.mipLodBias = 0.0,
.mipLodBias = lod_bias,
.pNext = sampler_next_struct,
};

Expand Down
Loading