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

Live updates for materials #342

Merged
merged 8 commits into from
Jun 7, 2022

Conversation

neph1
Copy link
Contributor

@neph1 neph1 commented May 26, 2022

I managed to break out this as well. Still one annoying "whitespace" diff in this one.
I made a preliminary merge test with these PR's into the big one and it was fairly smooth, so I'm less worried about that now.

neph1 added 4 commits May 25, 2022 18:50
AssetManager static hack to load textures in preview...
Conflicts:
	jme3-core/src/com/jme3/gde/core/editor/nodes/Diagram.java
	jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/ShaderNodeDiagram.java
	jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/previews/ColorPreview.java
	jme3-materialeditor/src/com/jme3/gde/materials/MaterialPreviewRenderer.java
matToRemove.clearParam(matParam.getName());
}

}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what caused the mat previews not to update when changing parameters. They got overwritten when the material was reloaded.

@neph1 neph1 marked this pull request as ready for review May 26, 2022 05:58
Copy link
Member

@MeFisto94 MeFisto94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general looking very good, just a few minor considerations

public Material reloadMaterial(Material mat) {
Material dummy;
try {
((ProjectAssetManager) mat.getMaterialDef().getAssetManager()).clearCache();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we could probably use the same technique as in L94, if I'm not mistaken (that is, if we can get an asset key from Material#getMaterialDef), because clearing the whole cache has bigger side effects (such as every j3o having to be reloaded from disk etc), If I still know that right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, but perhaps we can do that in a separate PR since I haven't changed any of that code?

@neph1
Copy link
Contributor Author

neph1 commented May 26, 2022

Yeah, those comments are from before. Btw, you can select the cog wheel and "hide whitespace" to only see the relevant changes.

@neph1 neph1 merged commit 3e1deeb into jMonkeyEngine:master Jun 7, 2022
@neph1 neph1 deleted the live_updates_for_materials branch May 6, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants