justingardner / mgl

A suite of mex/m files for displaying psychophysics stimuli
http://justingardner.net/mgl
Other
18 stars 22 forks source link

texture alignment to 256 #55

Closed justingardner closed 2 years ago

justingardner commented 2 years ago

Want to check-in that this works as expected. i.e. the padding should be invisible to the user of mglCreateTexture. Two things, I want to make sure of:

1) If the user creates a texture, say, 800 x 600 and it pads out - then when displaying in exact pixel resolution, it should show an 800x600 texture (i.e. the pad should have alpha 0) 2) If the user creates a texture and then scales it - the scale should reflect the size of the image they created -with the padding removed. this should work in both visual angle and pixel coordinates, but to be clear what I mean, in pixel coordinates, if I create a texture that is say 13x13 pixels and it pads out to align to 256, let's say that pads out to 16x16 (not sure if that's actually the case, but just for clarity). Then if I mglBlt the texture at 13x13 it will display the original texture of 13x13 pixels. Or if I blt to 26x26 each pixel should be doubled in each dimension (i.e. it should ignore the padding part).

Does that make sense? If these are all true, then we can drop the warning message from mglMetalCreateTexture as it's a backend implementation issue that shouldn't affect any users and there's no reason to warn.

benjamin-heasly commented 2 years ago

Maybe clip UV coordinates to match padding, instead of always using 0-1.

benjamin-heasly commented 2 years ago

I think this makes sense and it may be resolved as of commit e578d339ced69c0474d2a33b9ee91c1ae69fa17d

I added a test mglTestTextureOddSizes . This makes some small, odd-sized textures and positions and scales them so they should be neatly aligned and packed, according to the nominal sizes, like 13 pixels.

With the original padding code, there were unexpected gaps, which I think reproduces the concern you're pointing out above.

with-padding-out

As a start I tried just removing the padding code. This makes the alignment and packing look correct to me.

no-padding-out

In fact, the other tests in mglRunRenderingTests all seemed to run fine without the padding code. Perhaps we don't need this anymore (has Metal leveled up?). Or perhaps I'm missing something and we should add it back, in which case I'd need to understand the issue better.

Optimistically, I'll close this for now.

benjamin-heasly commented 2 years ago

OK, learning more from testing on a different machine with AMD graphics (Gardner lab lagavulin). I got this error that I hadn't seen on my own M1 iMac, running mglTestText:

​​Application Specific Information: _mtlValidateStrideTextureParameters:1824: failed assertion `Texture Descriptor Validation Linear texture: bytesPerRow (2336) must be aligned to 256 bytes '

So the alignment requirement is platform-specific. I finally found some docs where this mentioned, here under Linear Texture Restrictions. This says linear texture, but it seems to apply for 2D textures as well.

So strictly we should be checking a Metal device method like minimumLinearTextureAlignment.

I'll reopen this and see if we can do this automatically in the mglMetal app, or if we need to re-add the code in Matlab and work around the resizing with UV coordinates or similar.

benjamin-heasly commented 2 years ago

This SO post looks similar, feels like this might be on the right track https://stackoverflow.com/questions/62449741/how-to-set-up-byte-alignment-from-a-mtlbuffer-to-a-2d-mtltexture

benjamin-heasly commented 2 years ago

OK, I implemented the texture padding out to alignment in the mglMetal app. This way we can access the system-dependent minimumLinearTextureAlignment where/when we need it. It seems metal still respects the nominal texture size and ignores the padding when blt-ing textures, and the tests look good to my eye (same as above).

So I'll close this again.