J3MLoader always set generate mips flag to true regardless of whether it is required by MinFilter or not.
for example, NearestNoMipMaps and BilinearNoMipMaps do not require mips.
Shouldn’t it check MinFilter and generate mips only if it is required?
Because of that, J3MLoader is broken, I noticed if I set the min filter to BilinearNoMipMaps on texture and export the material to a j3m file in the code then if I load it at runtime the min filter is set to Trilinear. It will work fine if I set it to something other than BilinearNoMipMaps.
That is because BilinearNoMipMaps is not written to the j3m file as it is the default min filter in the Texture class and is supposed to be used when nothing is specified for the min filter in the j3m file:
but it does not work as expected because J3MLoader will set the mips generation flag to true and this causes TextureProcessor to change the min filter to Trilinear on texture when loaded with asset manager:
If someone manually adds the BilinearNoMipMaps inside j3m file then it will work fine and will not get changed to Trilinear after loading but the generate mips flag will still be set to true.
Solution:
Those can be fixed by checking this inside the min filter and setting the flag only if it is required.
We would need to override the applyToTextureKey method :
@Override
public void applyToTextureKey(final String option, final TextureKey textureKey) {
Texture.MinFilter minFilter = Texture.MinFilter.valueOf(option);
textureKey.setGenerateMips(minFilter.usesMipMapLevels());
}
May i ask what is not working?
I don’t get whats the wrong outcome, beside the flag set. (Which is not used to check if mips need to be generated on the gpu)
IMHO the filter used should also not be stored in the texture/image, it depends on the material/technique i use if i want filtration or not.
That said:
checks if mipLevels are set and if ‘needGeneratedMips’ is true
if true it generates the mips, and set ‘mipsWereGenerated’ to true
All i am saying is that ‘mipsWereGenerated’ is never used to check if mips should be generated or not. It is used only to disable the “not power of two” check in GLRenderer line 2640. (Don’t know how you include your source views here)
I have to check what happens when you change the filter later in code. Since there is no flag, indicating that mips are already generated, i fear that if the mips are not generated the first time a image is used, they are not generated at all, even if you change to trilinear later.
IMHO filtration type should be set as mat param in the j3md file and not be part of the j3m file. As a shader programmer i have control over the first one, but no control of the second one.
-When mips are not generated on load, the image gets re-uploaded in case you want them later
If you load a texture yourself, and not trough j3m loader, setGenerateMips is also set to true. So it is matching behavior.
Once mips are generated, you can clear the data buffer in the image and change the filters as needed. The texture still works.
If you do not generate the mips this will not work as soon as you use a mip requiring filter of course since it gets re-uploaded.
For me the most correct solution would be that the exporter writes the filter mode to the material file. Then a loaded model would render the same way as the stored one.
But as someone using the shader, I can pass whatever texture I want with whatever filtering I want as a material parameter. The MatDef shouldn’t care what sampling is used. It’s where you specify the specific texture that the filtering should be specified… just like UV repeat.
It is only if you load a texture with a name and do not use a TextureKey.
generateMips is off by default in TextureKey.
What will not work?
Yes, it will render correctly in that case, but are you saying it should always generate mips and upload them to GPU regardless of what is the MinFilter (at load time)? Or am I missing something?
Afaik, it is the needGeneratedMips flag that is used to decide to generate mips on GPU.
But probably you are right, we might need to add a check for it as well here so it does not regenerate it on GPU if it has generated once.
But how many times does the GLRenderer.updateTexImageData() is called for a texture? Isn’t it when it is uploaded to GPU for the first time and once its data is changed? In which case the mipsWereGenerated flag will be already invalidated.
On GitHub shift select the lines you want to view and use “Copy Permalink”.
If i clear the buffer data before mips have been generated, then use a filter requireing mips, the texture will black out.
I do not agree. Same with the UV repeat. As material provider i know how i access the textures. If i use textureLod i would need the texture to have mips.
Same with uv’s. How would i as material user know how the texture is accessed.
I guess that the first time a terrain shader is used i had always go back to code and modify my uv repeats.
I do not see a usecase where i want my red car mipmapped and my brown truck not.
I can use Lighting.j3md to draw Minecraft like worlds by setting no mips and Nearest for the filter. I can use Lighting.j3md to render realistic scenes by allowing MIPS.
I can use lighting.j3md to render a texture that repeats in U but not V. I can use Lighting.j3md to render a texture that repeats in both U and V.
Lighting.j3md does not care what texture I sent it.
You can, and it is perfectly fine if the shader author decided that it can cope with everything you throw in.
For me its the same reason we have mat param overrides and forced renderstates at a texture level. Sometimes i need the stuff setup in a way i require it.
(Kind of offtopic here)
(Arguing with Paul always makes me think i have to be wrong, but in this case i think i really want “forced texture parameters” but at texture level, we don’t have it, so i cope with it in code as good i can)
To the topic here, the issues seems to come down to:
a) filter type not gets stored when exporting the j3m.
b) When not importing trough texturekey manually mips are always generated.
to a: i think it really should be stored
to b: don’t know. i like my mips. and i like to clear the image data after loading so i want my mips to be generated in the first place
They already are stored, except for BilinearNoMipMaps because that is supposed to be the default when nothing is specified.
But when I specify MinNearestNoMipMaps or MinBilinearNoMipMaps I explicitly mean that I do not want mips to be generated, it does not make sense to me to clear it afterward in my code.
Also, I do not know how you would clear it from GPU memory but even if you do, it will be regenerated again when setUpdateNeeded flag is set on the image.
If you use textureLod() with no mipmaps then it’s supposed to clamp it to the number of levels available…which will be 0. So the function should still work it just may not look as good as it could.
…which may be a tradeoff that the user of the shader wants to make if they are trying to save RAM.
I understand what you are saying but the cases I see for requiring a specific type of texture filtering, UV repeat, etc. at the SHADER level seem really small… especially given how loose the whole material parameter thing is to begin with. (Edit: for all of my own use-cases, the type of texture has way more to do with the mesh I’m using.)
Seems like something that could be handled in the documentation that comes with the shader: for best results, use textures like “this”.
With clear i was ment that i want my whole image data gone from ram for static textures after i uploaded them to gpu.
In this particular texture instance of the image yes.
In your case it should be set to the filter you want, no question there.
I personally do not want a reupload and mip generation during rendering only because i change the filter method. But it might be only me because i preload my whole level to make sure i don’t have to do it during a game session