GL_UNPACK_ALIGNMENT? 1 or 4 or no care? Also: GL_LINEAR?

[m-7761]

Should GL_UNPACK_ALIGNMENT be set? Power-two textures I think happen to appear to be 4 even when they are 1. NPOT textures need to set to 1.

These use glTexImage2D/gluBuild2DMipmaps:

Model::loadTextures ModelViewport TextureWidget

The Texture class's data looks 1 packed. I think these should just set to match Texture probably.

EDITED glPixelStorei(GL_UNPACK_ALIGNMENT,1);

EDITED: See also gluBuild2DMipmaps issue below.

[zturtleman]

Yeah, it should probably be set but because of RGB data not NPOT specifically.

8-bit per-channel RGBA data is always 4 byte aligned. 8-bit per-channel RGB data may need row alignment set to 1 byte (GL_UNPACK_ALIGNMENT 1) depending on the width. Coincidentally RGB power of two textures have 4 byte alignment in most cases (3 bytes × 4 pixels) but 2x2 RGB has second row start at byte 6 (3 bytes × 2 pixels) which is 2 byte aligned.

A hack for power of two only textures would be not converting 2x2 RGBA to RGB in Texture::removeOpaqueAlphaChannel(). The performance difference (if any) of using GL_UNPACK_ALIGNMENT 1 is probably irrelevant for a model editor though.

It's unclear if uploading RGB data is actually beneficial. It's suggested that OpenGL drivers convert RGB to RGBA source. In which case, always using RGBA would allow using the default GL_UNPACK_ALIGNMENT 4. (Which is what Quake 3 and my own Clover's Toy Box renderer do.)

[m-7761]

The performance difference (if any) of using GL_UNPACK_ALIGNMENT 1 is probably irrelevant for a model editor though.

It's not a matter of performance... it's just a convenience function... meaning that if it didn't exist data would have to be reformatted before it could be transferred, which would be itself a performance problem (of course OpenGL would go and foil that goal by mandating its images upside down... not providing a corresponding option, or taking a (possibly negative) stride argument in the texture function, which, which would be most sane.)

As for uploading RGB, the unpacking should be done on the video hardware, and in general anything that can make the transfer smaller is a gain. So RGB eliminating 8bits per pixel should improve transfer speeds. Memory transfer is always the bottleneck by order of magnitude or more. I imagine that any conversion like that entails no performance penalty, being R to RGBA or whatever.

[m-7761]

P.S. I experienced huge load time problems when I first started loading models, mainly because of the removeOpaqueAlphaChannel you mentioned. I had to profile the program, which is something I don't know if I've ever needed to do before. In general I think it's unnecessary... but the problem with it is I'd removed some "malloc" and "free" code and "strdup" too around Texture, and replaced the data pointer with a vector, so that the algorithm for scanning the RGBA data was getting bound-checked by debug STL code. I couldn't imagine the checking would kill performance so hard... which makes it pretty pointless since you have to write code around it to disable it anyway.

I think compilers should run optimization on internal bound check code even when optimization is disabled (debug) so that the difference is less dramatic in trivial loops.

[m-7761]

Unrelated, but adjacent, I think since Model::loadTextures is using gluBuild2DMipmaps it should probably be using one of the mipmap min-filters:

/ TextureMinFilter / / GL_NEAREST / / GL_LINEAR /

define GL_NEAREST_MIPMAP_NEAREST 0x2700

define GL_LINEAR_MIPMAP_NEAREST 0x2701

define GL_NEAREST_MIPMAP_LINEAR 0x2702

define GL_LINEAR_MIPMAP_LINEAR 0x2703

I'm no expert, but I think GL_LINEAR may not automatically utilize mipmaps.

I started scrutinizing this because I'm having an issue (https://community.khronos.org/t/gl-nearest-frayed-edges-rounding-in-simple-image-editor/104696) with GL_NEAREST in the UV editor. I only mention it in case you're seeing the same issue.

https://github.com/zturtleman/mm3d/blob/dcb15f3440a62376ed2e09d01960b58426c13ef6/src/libmm3d/model_texture.cc#L110