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

Texture.load() doesn't trigger mipmap generation #2132

Closed
oxplay2 opened this issue Oct 26, 2023 · 2 comments · Fixed by #2134
Closed

Texture.load() doesn't trigger mipmap generation #2132

oxplay2 opened this issue Oct 26, 2023 · 2 comments · Fixed by #2134
Labels
defect Something that is supposed to work, but doesn't. Less severe than a "bug"
Milestone

Comments

@oxplay2
Copy link

oxplay2 commented Oct 26, 2023

initially: Simsilica/JmeConvert#4

I still might be wrong, but it seems like an issue.

Texture.java

    public void setMinFilter(MinFilter minificationFilter) {
        if (minificationFilter == null) {
            throw new IllegalArgumentException(
                    "minificationFilter can not be null.");
        }
        this.minificationFilter = minificationFilter;
        if (minificationFilter.usesMipMapLevels() && image != null && !image.isGeneratedMipmapsRequired() && !image.hasMipmaps()) {
            image.setNeedGeneratedMipmaps();
        }
    }

That is executed always first time by:

    public void setImage(Image image) {
        this.image = image;

        // Test if mipmap generation required.
        setMinFilter(getMinFilter());
    }

GLTFLoader execute it second time for setting proper filtering by:

        if (minFilter != null) {
            texture.setMinFilter(minFilter);
        }

BUT J3O importer when using texture.read capsule do not execute it second time, causing not setting proper bollean for generateMipMaps.

Here is texture.read:

    public void read(JmeImporter importer) throws IOException {
        InputCapsule capsule = importer.getCapsule(this);
        name = capsule.readString("name", null);
        key = (TextureKey) capsule.readSavable("key", null);

        // load texture from key, if available
        if (key != null) {
            // key is available, so try the texture from there.
            try {
                Texture loadedTex = importer.getAssetManager().loadTexture(key);
                image = loadedTex.getImage();
            } catch (AssetNotFoundException ex){
                Logger.getLogger(Texture.class.getName()).log(Level.SEVERE, "Cannot locate texture {0}", key);
                image = PlaceholderAssets.getPlaceholderImage(importer.getAssetManager());
            }
        }else{
            // no key is set on the texture. Attempt to load an embedded image
            image = (Image) capsule.readSavable("image", null);
            if (image == null){
                // TODO: what to print out here? the texture has no key or data, there's no useful information ..
                // assume texture.name is set even though the key is null
                Logger.getLogger(Texture.class.getName())
                        .log(Level.SEVERE, "Cannot load embedded image {0}", toString());
            }
        }

        anisotropicFilter = capsule.readInt("anisotropicFilter", 1);
        minificationFilter = capsule.readEnum("minificationFilter",
                MinFilter.class,
                MinFilter.BilinearNoMipMaps);
        magnificationFilter = capsule.readEnum("magnificationFilter",
                MagFilter.class, MagFilter.Bilinear);
    }

and after setting minificationFilter this do not execute same glued code as setMinFilter() do.

@JohnLKkk
Copy link
Contributor

You are right, this is what caused incorrect texture filtering + mipmapping results when rendering j3o. I tested it locally.

see:https://hub.jmonkeyengine.org/t/gltf-to-j3o-texture-filtering-issue/47197/73

JohnLKkk added a commit to JohnLKkk/jmonkeyengine that referenced this issue Oct 27, 2023
JohnLKkk added a commit to JohnLKkk/jmonkeyengine that referenced this issue Oct 27, 2023
JohnLKkk added a commit to JohnLKkk/jmonkeyengine that referenced this issue Oct 27, 2023
@JohnLKkk
Copy link
Contributor

I have fixed it here:#2134

@stephengold stephengold changed the title Texture.java setMinFilter() contains glued code should be also executed by texture.read() Texture.load() doesn't trigger mipmap generation Oct 27, 2023
@stephengold stephengold added the defect Something that is supposed to work, but doesn't. Less severe than a "bug" label Oct 27, 2023
@stephengold stephengold added this to the Future Release milestone Oct 27, 2023
stephengold pushed a commit that referenced this issue Oct 30, 2023
riccardobl pushed a commit that referenced this issue Nov 9, 2023
* Fixes Issue #2132

* jme3-core:Adding support for Specular-AA

* jme3-core:del Specular_Anti_Aliasing2

* jme3-core:update Specular_Anti_Aliasing

* jme3-core:update Specular_Anti_Aliasing

* jme3-core:add MatParam:Sigma,Kappa

* jme3-terrain:Adding support for Specular-AA

* jme3-core:update PBRLighting.j3md(with SpecularAA)

* jme3-terrain:update PBRTerrain.j3md(with SpecularAA)

* jmee-core:remove the "f" suffix

* jmee-terrain:remove the "f" suffix

* jme3-core:Updated parameter names in PBRLighting.j3md

* jme3-terrain:Updated parameter names in PBRTerrain.j3md/AdvacedPBRTerrain.j3md

* jme3-core:Updated macro definitions in PBRLighting.j3md

* jme3-terrain:Updated macro definitions in PBRTerrain.j3md/AdvancedPBRTerrain.j3md

---------

Co-authored-by: chenliming <liming.chen@yingxiong.com>
@stephengold stephengold modified the milestones: Future Release, v3.7.0 Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Something that is supposed to work, but doesn't. Less severe than a "bug"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants