Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

SamplerStates aren't properly applied when active texture changes #842

Closed
elisee opened this Issue · 4 comments

3 participants

@elisee

I haven't gotten to the bottom of this one yet, but basically sampler states aren't being applied properly when the active texture has changed.

I use SampleState.PointClamp for rendering CraftStudio's low-poly, pixelated models.

When I change the texture bound to my "textureSampler" parameter (which should be called Texture, but that is another story ;) #641), and then apply my effect, the sampler state isn't properly applied and I get the default interpolation filter (linear & wrap I think) instead of what I requested (point & clamp).

This means that the very first model to be rendered in my scene (with texture 1) has proper texture sampling, and then all subsequent models (with textures 2, 3, 4 ...) are linearly interpolated.

Disabling the _dirty checks in SamplerStateCollection.SetSamplers() fixes the issue, so my guess is the dirty bit flag isn't being set when it should in some cases. I'll report back later if I find a proper fix.

(Obviously this works correctly in XNA)

@shilrobot

I ran into this myself, did some investigating. The problem is mostly in SamplerState.Activate and SamplerStateCollection.SetSamplers.

The main problem is that, unlike D3D where sampler state is stored "on the device", in GL the sampler state is specific to a texture object. The current code doesn't seem to be aware of this.

So basically if you do this:

  1. Set device to use SamplerState.PointClamp (assuming this is different than the old state)
  2. Render something with Texture A
  3. Render something with Texture B

Then the draw with texture A is correct, while the draw with texture B isn't. This is because the new state got applied to Texture A, the dirty flag got cleared, then Texture B still retained its old sampling state. You also can't do this:

  1. Set device to use SamplerState.PointClamp
  2. Render something with Texture A
  3. Set device to use SamplerState.PointClamp
  4. Render something with Texture B

...because the second sampler state change is considered a "no op" (because it matches the old cached value.)

What should probably happen is that it should cache the last used sampler state on the Texture instance, and then only make calls to GL if the requested state at the time of drawing does not match the state we last set the Texture to use.

The code is also not capable of dealing with multiple textures per draw, and will break in bizarre ways if you try to use it for that. This could be fixed by just calling glActivateTexture at the right place in SamplerStateCollection.SetSamplers(). If you wanted to truly minimize the amount of necessary GL calls, this might need to be interleaved with TextureCollection.SetTextures().

Even with those things fixed, there's the possibility that you try to use the same texture for two texture units, with different sampler states. I believe this is possible in D3D but not in GL, not without cloning the texture, or possibly some extension. Luckily I don't think this case gets that much use.

I'll try to put together a patch if I have time...

@shilrobot

It should also probably also throw an exception if you modify any of the state objects once they've been used once (like XNA does.) That would prevent people only using MonoGame from running into issues. Currently in MonoGame you can modify state objects willy-nilly even after they are used, which will confuse the heck out of state change detection.

I don't think it'd be a significant performance change, since you shouldn't have any calls that modify the properties of already-used state objects to begin with.

@tomspilman
Owner

@shilrobot

Good catch... indeed that is the problem.

If you wanted to truly minimize the amount of necessary GL calls,
this might need to be interleaved with TextureCollection.SetTextures().

That might just be the best solution here. If I get time i'll try it.

there's the possibility that you try to use the same texture for two
texture units, with different sampler states.

Yep... another issue.

We should try to detect and assert in that case.

Fixing it might not be worth the trouble. There is ARB_sampler_objects, but I don't think it works for mobile. Maybe GLES 3.0 offers a solution, but I've not looked.

@shilrobot

I put up a first attempt at a fix, see pull request #889

@tomspilman tomspilman closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.