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

Port to blender4 Merged with blender4-image-export #296

Merged
merged 14 commits into from
Jul 16, 2024

Conversation

Spiderguy-F
Copy link
Contributor

Opened a new pull request, like you suggested

@keianhzo keianhzo self-requested a review June 11, 2024 08:04
@keianhzo keianhzo changed the base branch from blender4-image-export to master June 11, 2024 16:21
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

As talked in the meeting, we need to correctly handle the return parameters from __gather_texture_transform_and_tex_coord and we should be good to land.

I think probably something like this would work:

if bpy.app.version < (3, 2, 0):
    tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
        texture_socket, export_settings)
elif bpy.app.version >= (4, 0, 0):
    socket = gltf2_blender_search_node_tree.NodeSocket(
        texture_socket, blender_material)
    tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
        socket, export_settings)
else:
    tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
        texture_socket, export_settings)

@keianhzo keianhzo self-requested a review June 18, 2024 16:57
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

Added updates as commit suggestion

else:
tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
print("processing __gather_texture_transform_and_tex_coord")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("processing __gather_texture_transform_and_tex_coord")

else:
tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
print("processing __gather_texture_transform_and_tex_coord")
tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
texture_socket, export_settings)

Copy link
Contributor

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

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

@keianhzo 4.0 support turned out to be trickier than we thought at the dev meetup.

@Spiderguy-F We identified at the meetup that Blender 4.0 was the cause of the errors I had been seeing. I have added suggestions to fix all the errors and so now the add-on should work for all the supported Blender versions. I have incorporated the fixes suggested by @keianhzo in my suggestions. It turns out that Blender 4.0 requires the same code path as versions below 3.2, but I don't see a cleaner way to implement this than what I've suggested here; however, anyone is welcome to try to improve on my suggestions. :)

@@ -47,7 +49,10 @@ def from_blender_image(image: bpy.types.Image):

def encode(self, mime_type: Optional[str], export_settings) -> Union[Tuple[bytes, bool], bytes]:
if mime_type == "image/vnd.radiance":
return self.encode_from_image_hdr(self.blender_image())
if bpy.app.version < (4, 0, 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if bpy.app.version < (4, 0, 0):
if bpy.app.version < (4, 1, 0):

Comment on lines 374 to 377
elif bpy.app.version >= (4, 0, 0):
socket = gltf2_blender_search_node_tree.NodeSocket(texture_socket, blender_material)
tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
socket, export_settings)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
elif bpy.app.version >= (4, 0, 0):
socket = gltf2_blender_search_node_tree.NodeSocket(texture_socket, blender_material)
tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
socket, export_settings)
elif bpy.app.version < (4, 0, 0):
tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
texture_socket, export_settings)
elif bpy.app.version < (4, 1, 0):
tex_transform, tex_coord = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
texture_socket, export_settings)

addons/io_hubs_addon/io/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

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

Left a couple slight changes for 4.0 support, but otherwise it looks good to me.

@@ -10,6 +10,8 @@
from io_scene_gltf2.blender.exp import gltf2_blender_gather_texture_info, gltf2_blender_export_keys
from io_scene_gltf2.blender.exp import gltf2_blender_image
from io_scene_gltf2.blender.exp.gltf2_blender_gather_cache import cached
if bpy.app.version >= (4, 0, 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if bpy.app.version >= (4, 0, 0):
if bpy.app.version >= (4, 1, 0):

else:
tex_transform, tex_coord, _ = gltf2_blender_gather_texture_info.__gather_texture_transform_and_tex_coord(
texture_socket, export_settings)
socket = lightmap_node.inputs.get("Lightmap") if bpy.app.version < (4, 0, 0) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
socket = lightmap_node.inputs.get("Lightmap") if bpy.app.version < (4, 0, 0) \
socket = lightmap_node.inputs.get("Lightmap") if bpy.app.version < (4, 1, 0) \

Copy link
Contributor

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

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

LGTM. Merging.

@Exairnous Exairnous merged commit 0ed665d into Hubs-Foundation:master Jul 16, 2024
6 checks passed
@GottfriedHofmann GottfriedHofmann deleted the PortToBlender4 branch July 21, 2024 07:40
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.

4 participants