Skip to content
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.

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

Fix FillStyleExtension artifact #8668

Merged
merged 2 commits into from
Mar 18, 2024
Merged

Fix FillStyleExtension artifact #8668

merged 2 commits into from
Mar 18, 2024

Conversation

Pessimistress
Copy link
Collaborator

@Pessimistress Pessimistress commented Mar 16, 2024

Default texture parameters sets mipmapFilter: 'linear' and it cannot be unset by merging with another SampleProps

Screen Shot 2024-03-16 at 10 06 20 AM

Alternatively we could remove it from the defaults but that will be a bigger breaking change.

Change List

  • Work around and comment
  • Restore render tests

@coveralls
Copy link

coveralls commented Mar 16, 2024

Coverage Status

coverage: 86.454% (+0.03%) from 86.423%
when pulling 0fc6506 on x/fill-extension-fix
into aa49b23 on master.

minFilter: 'linear'
// Override default mipmap filter 'linear', i.e. set MIN_FILTER to LINEAR instead of LINEAR_MIPMAP_LINEAR
// @ts-expect-error invalid value for type `SamplerProps` - luma.gl should allow unset
mipmapFilter: ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would be better to explicitly support mipmapFilter: 'none' in luma to make it clearer what is happening. None of the other SamplerProps allow falsy values

Copy link
Collaborator

@donmccurdy donmccurdy Mar 18, 2024

Choose a reason for hiding this comment

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

I think lodMaxClamp: 0 would have the effect of disabling mips, if that's the goal? Luma is mirroring the WebGPU spec, in which all samplers have mipmapFilter, but you can restrict the range of mipmaps used with lodMinClamp / lodMaxClamp.

Opened PR:

@Pessimistress Pessimistress merged commit 2730378 into master Mar 18, 2024
4 checks passed
@Pessimistress Pessimistress deleted the x/fill-extension-fix branch March 18, 2024 11:33
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.

4 participants