Skip to content

Conversation

capdevon
Copy link
Contributor

@capdevon capdevon commented Jun 10, 2025

This PR introduces the capability to correctly serialize and deserialize the receivesShadows boolean field within the Material class. Previously, this variable's persistence was overlooked during material saving and loading, despite its documented purpose as a marker for model loaders to indicate shadow reception for geometries. I've leveraged the existing setReceivesShadows functionality, which, as its Javadoc states, is a valuable marker that was not fully utilized in the serialization process.

Key changes include:

  • Updated Material.java to read and write the receivesShadows field during serialization and deserialization.
  • Modified J3MRootOutputCapsule.java to properly handle and include the receivesShadows property in the exported .j3m file format.
  • Updated TestMaterialWrite.java with a test case to ensure the receivesShadows value is correctly persisted and loaded.
// from Material.java
   /**
     * Set if the material should receive shadows or not.
     *
     * <p>This value is merely a marker, by itself it does nothing.
     * Generally model loaders will use this marker to indicate
     * the material should receive shadows and therefore any
     * geometries using it should have {@link com.jme3.renderer.queue.RenderQueue.ShadowMode#Receive} set
     * on them.
     *
     * @param receivesShadows if the material should receive shadows or not.
     */
    public void setReceivesShadows(boolean receivesShadows) {
        this.receivesShadows = receivesShadows;
    }

Edit:
Crucially, this modification is fully backward-compatible with existing .j3m files. Materials saved with older versions of the engine that didn't persist receivesShadows will still load without issues, and the field will simply default to its initial value (typically false) as if it were never explicitly set.

Test passed:

Material TestMaterial : Common/MatDefs/Light/Lighting.j3md {

    Transparent On

    ReceivesShadows On

    MaterialParameters {
      Diffuse : 1.0 1.0 1.0 1.0
      UseMaterialColors : true
      ParallaxHeight : 0.05
      Ambient : 0.2 0.2 0.2 1.0
      BackfaceShadows : false
      AlphaDiscardThreshold : 0.5
      DiffuseMap : Flip WrapRepeat_S WrapMirroredRepeat_T MinBilinearNoMipMaps MagNearest "Common/Textures/MissingTexture.png"
      Shininess : 2.5
    }

    AdditionalRenderState {
      PointSprite On
      DepthWrite Off
      ColorWrite Off
      PolyOffset -1.0 1.0
      DepthTest Off
      Blend Alpha
      Wireframe On
      LineWidth 5.0
    }
}

@yaRnMcDonuts
Copy link
Member

yaRnMcDonuts commented Jun 11, 2025

I'm slightly confused what this value would be used for, since the comment says:

 <p>This value is merely a marker, by itself it does nothing.
     * Generally model loaders will use this marker to indicate
     * the material should receive shadows and therefore any
     * geometries using it should have {@link com.jme3.renderer.queue.RenderQueue.ShadowMode#Receive} set
     * on them.
     *

So does this mean it is just a boolean value that gets flagged automatically on a material so that the shader/material can easily know if shadows are enabled or not? And by default, this does nothing else in the engine?

If so then I could see cases where it could be useful to know if shadows are being recieved in the shader to potentially adjust brightness (if you want to avoid players turning off shadows to get an advantage from a brighter scene, for example). But I'm curious to know what else you have found it to be useful for as well.

(the only issue I see here is in cases where there's a single material that is used by more than 1 geometry where the geometries are using different shadow modes, but I suspect that is probably unlikely to happen)

Or am I completely misunderstanding this?

@yaRnMcDonuts yaRnMcDonuts added this to the v3.9.0 milestone Jun 11, 2025
@capdevon
Copy link
Contributor Author

capdevon commented Jun 11, 2025

The primary goal of this change is to bring consistency to the Material class's persistence API. Just like the isTransparent field, receivesShadows is a boolean attribute of a material, and it was previously completely ignored during serialization. By making receivesShadows persistable, we ensure that the entire state of the Material object, including all its attributes, is consistently saved and loaded, providing a more robust and predictable API.

The potential use that I have seen is not at all about shaders, as you imagined. IMO according to the javadoc for the setReceivesShadows() method, it could be used as follows:

     // e.g. String matDef = "Common/Materials/RedColor.j3m"

    private Geometry makeShape(String name, Mesh mesh, String matDef) {
        Geometry geo = new Geometry(name, mesh);
        Material mat = assetManager.loadMaterial(matDef);
        geo.setMaterial(mat);
        if (mat.isTransparent()) {
            geo.setQueueBucket(RenderQueue.Bucket.Transparent);
        }
        if (mat.isReceivesShadows()) {
            geo.setShadowMode(RenderQueue.ShadowMode.Receive);
        }
        return geo;
    }

Edit:
This approach allows the appearance and configuration of transparencies, shadows, and other visual elements to be driven directly from the .j3m file. This unlocks significant potential.

Here is another example of applying the same material to all the geometries of an entire scene by reducing the number of allocated objects and configuring transparencies and shadows via .j3m files:

    // e.g. String matDef = "Common/Materials/RedColor.j3m"

    private void updateMaterial(Spatial scene, String matDef, Predicate<Spatial> filter) {
        // All geometries use the same instance of Material.
        final Material mat = assetManager.loadMaterial(matDef);

        scene.depthFirstTraversal(new SceneGraphVisitorAdapter() {
            public void visit(Geometry geom) {
                if (filter.test(geom)) {
                    geom.setMaterial(mat);
                    if (mat.isTransparent()) {
                        geom.setQueueBucket(RenderQueue.Bucket.Transparent);
                    }
                    if (mat.isReceivesShadows()) {
                        geom.setShadowMode(RenderQueue.ShadowMode.Receive);
                    }
                }
            }
        });
    }

And by default, this does nothing else in the engine?

exactly, it's just a simple boolean marker, it doesn't do anything in the engine

@capdevon
Copy link
Contributor Author

capdevon commented Jun 20, 2025

Is there any doubt about this PR? I would need it merged to implement this other fix #2497

@yaRnMcDonuts
Copy link
Member

yaRnMcDonuts commented Jun 23, 2025

I still haven't done a full review of all the code in this PR yet, but based on my initial look through it sounds like this PR's concept is okay and should be good to merge soon, most likely right before I make the next alpha2 release at the very end of June.

I'm going to post a handful of PRs (including this one) to the jme 3.9 thread to hopefully get as many reviewed and merged in time for that alpha2 release. But many will likely end up waiting until the next alpha since there's currently so many open.

So also feel free to post a few of your other PRs to that thread if there's any in particular that you'd want to have reviewed and merged sooner to have a better chance of being included in alpha2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants