-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Texture Path Preservation #2203
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
Changes from 7 commits
f3d799b
ce6eb19
78856cf
1f1afcc
3bef05c
118ed07
d8107ff
c2837df
bec733c
bc7250c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,7 +112,8 @@ class Config: | |
| def __init__(self, **data): | ||
| super().__init__(**data) | ||
|
|
||
| if not (self.image_path is None) ^ (self.image_array is None): | ||
| # Allow both image_path and image_array, but require at least one | ||
| if self.image_path is None and self.image_array is None: | ||
|
Comment on lines
-115
to
+116
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it makes sense. |
||
| gs.raise_exception("Please set either `image_path` or `image_array`.") | ||
|
|
||
| if self.image_path is not None: | ||
|
|
@@ -121,17 +122,21 @@ def __init__(self, **data): | |
| self.image_path = os.path.join(gs.utils.get_assets_dir(), self.image_path) | ||
|
|
||
| if not os.path.exists(self.image_path): | ||
| gs.raise_exception( | ||
| f"File not found in either current directory or assets directory: '{input_image_path}'." | ||
| ) | ||
| if self.image_array is None: | ||
| # Only error if we don't have image_array as fallback | ||
| gs.raise_exception( | ||
| f"File not found in either current directory or assets directory: '{input_image_path}'." | ||
| ) | ||
|
|
||
| # Load image_path as actual image_array, unless for special texture images (e.g. `.hdr` and `.exr`) that are only supported by raytracers | ||
| if self.image_path.endswith(HDR_EXTENSIONS): | ||
| self.encoding = "linear" # .exr or .hdr images should be encoded with 'linear' | ||
| if self.image_path.endswith((".exr")): | ||
| self.image_path = mu.check_exr_compression(self.image_path) | ||
| else: | ||
| self.image_array = np.array(Image.open(self.image_path)) | ||
| # Only load image if we don't already have image_array | ||
| if self.image_array is None: | ||
| # Load image_path as actual image_array, unless for special texture images (e.g. `.hdr` and `.exr`) that are only supported by raytracers | ||
| if self.image_path.endswith(HDR_EXTENSIONS): | ||
| self.encoding = "linear" # .exr or .hdr images should be encoded with 'linear' | ||
| if self.image_path.endswith((".exr")): | ||
| self.image_path = mu.check_exr_compression(self.image_path) | ||
| else: | ||
| self.image_array = np.array(Image.open(self.image_path)) | ||
|
|
||
| elif self.image_array is not None: | ||
| if not isinstance(self.image_array, np.ndarray): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,11 +207,12 @@ def surface_uvs_to_trimesh_visual(surface, uvs=None, n_verts=None): | |
| uvs = uvs.copy() | ||
| uvs[:, 1] = 1.0 - uvs[:, 1] | ||
| assert texture.image_array.dtype == np.uint8 | ||
| material_kwargs = {"image": Image.fromarray(texture.image_array), "diffuse": (1.0, 1.0, 1.0, 1.0)} | ||
| if texture.image_path is not None: | ||
| material_kwargs["image_path"] = texture.image_path | ||
| visual = trimesh.visual.TextureVisuals( | ||
| uv=uvs, | ||
| material=trimesh.visual.material.SimpleMaterial( | ||
| image=Image.fromarray(texture.image_array), diffuse=(1.0, 1.0, 1.0, 1.0) | ||
| ), | ||
| material=trimesh.visual.material.SimpleMaterial(**material_kwargs), | ||
| ) | ||
| else: | ||
| # fall back to color texture | ||
|
|
@@ -499,9 +500,14 @@ def tonemapped(image): | |
| return (np.clip(np.power(image / 255 * np.power(2, exposure), 1 / 2.2), 0, 1) * 255).astype(np.uint8) | ||
|
|
||
|
|
||
| def create_texture(image, factor, encoding): | ||
| def create_texture(image, factor, encoding, image_path=None): | ||
| if image is not None: | ||
| return gs.textures.ImageTexture(image_array=image, image_color=factor, encoding=encoding) | ||
| return gs.textures.ImageTexture( | ||
| image_array=image, | ||
| image_path=image_path, | ||
| image_color=factor, | ||
| encoding=encoding, | ||
| ) | ||
|
Comment on lines
+503
to
+510
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same. This API is a bit broken I think. What about adding other method called |
||
| if factor is not None: | ||
| return gs.textures.ColorTexture(color=factor) | ||
| return None | ||
|
|
@@ -928,6 +934,7 @@ def create_plane(normal=(0.0, 0.0, 1.0), plane_size=(1e3, 1e3), tile_size=(1, 1) | |
| ), | ||
| material=trimesh.visual.material.SimpleMaterial( | ||
| image=Image.open(os.path.join(get_assets_dir(), "textures/checker.png")), | ||
| image_path="textures/checker.png", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like dark magic. This is not officially supported by Trimesh from what I know, but maybe I'm wrong. |
||
| ), | ||
| ) | ||
| else: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -510,6 +510,28 @@ def test_2_channels_luminance_alpha_textures(show_viewer): | |
| ) | ||
| ) | ||
|
|
||
| @pytest.mark.required | ||
| def test_plane_texture_path_preservation(show_viewer): | ||
| """Test that plane primitives preserve texture paths through the mesh pipeline.""" | ||
| scene = gs.Scene(show_viewer=show_viewer, show_FPS=False) | ||
| plane = scene.add_entity(gs.morphs.Plane()) | ||
| scene.build() | ||
|
|
||
| # Check that the plane's vgeom has a texture with a path | ||
| assert len(plane.vgeoms) > 0 | ||
| vgeom = plane.vgeoms[0] | ||
| assert vgeom.vmesh is not None | ||
| assert vgeom.vmesh.surface is not None | ||
|
|
||
| texture = vgeom.vmesh.surface.diffuse_texture | ||
| assert texture is not None | ||
| assert isinstance(texture, gs.textures.ImageTexture) | ||
|
Comment on lines
+520
to
+528
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand. These checks seem to be completely unrelated to what this test is pretending to check. Either broaden the scope of this unit test (test name, docstring, comments...) or remove these checks. |
||
|
|
||
| # The texture should have both image_path and image_array | ||
| assert texture.image_path is not None | ||
| assert texture.image_array is not None | ||
|
Comment on lines
+521
to
+532
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. those asserts are not helpful
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Either redundant or out of scope. |
||
| assert "checker.png" in texture.image_path | ||
|
|
||
|
|
||
| @pytest.mark.required | ||
| @pytest.mark.skipif(platform.machine() == "aarch64", reason="Module 'tetgen' is crashing on Linux ARM.") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend not supporting specifying both and raise an exception if it happens. Then load image files directly inside this method if provided.