blender/blender

UV islands display issue - macOS Metal #126542

New Issue

Closed

opened 2024-08-20 11:16:05 +02:00 by thibault-ging · 12 comments

thibault-ging commented

2024-08-20 11:16:05 +02:00

Operating system: Mac Studio (Sonoma 14.5)
Graphics card: Apple M1 Max

Broken: 4.2.0
Broken: 4.3.0 alpha (baa20eb289)
Worked:

When selecting part of a big mesh, in the UV editor faces appear light orange but visuals of edges are everywhere on the screen, can't precisely work that way.
Everything working as usual though; created UV vertices, edges and faces can be manipulated. Just a visual pain because I can not see them well under all that UV web.
Only on certains meshes, so I assume it might be an issue with big meshes only (I work for WebGL devs so 40k triangles is quite a large mesh in my case).

Exact steps for others to reproduce the error
Select part of the mesh (already selected when opening the .blend file).

Operating system: Mac Studio (Sonoma 14.5) Graphics card: Apple M1 Max Broken: 4.2.0 Broken: 4.3.0 alpha (baa20eb28976) Worked: When selecting part of a big mesh, in the UV editor faces appear light orange but visuals of edges are everywhere on the screen, can't precisely work that way. Everything working as usual though; created UV vertices, edges and faces can be manipulated. Just a visual pain because I can not see them well under all that UV web. Only on certains meshes, so I assume it might be an issue with big meshes only (I work for WebGL devs so 40k triangles is quite a large mesh in my case). **Exact steps for others to reproduce the error** Select part of the mesh (already selected when opening the .blend file).

uvDisplayIssue.blend

1.1 MiB

Capture d’écran 2024-08-20 à 10.30.06.png

4.8 MiB

Capture d’écran 2024-08-20 à 10.30.06.png

thibault-ging added the

labels 2024-08-20 11:16:06 +02:00

thibault-ging commented

2024-08-20 11:21:04 +02:00

Author

Has been like this for a long time (for all I can remember since I started working on this machine 1 year ago).

Has been like this for a long time (for all I can remember since I started working on this machine 1 year ago).

Alaska commented

2024-08-20 11:41:11 +02:00

Member

I can confirm this issue in Blender 4.2.0 with a M1 Pro. Can't on a Windows computer. It's likely to be a Metal issue.

I don't currently know the steps to reproduce this issue from scratch. @thibault-ging Can you provide steps to reproduce this issue from a new file?

I will set to high severity as it's a bad issue.

I can confirm this issue in Blender 4.2.0 with a M1 Pro. Can't on a Windows computer. It's likely to be a Metal issue. I don't currently know the steps to reproduce this issue from scratch. @thibault-ging Can you provide steps to reproduce this issue from a new file? I will set to high severity as it's a bad issue.

Alaska added

Viewport & EEVEE

and removed

labels 2024-08-20 11:41:53 +02:00

thibault-ging commented

2024-08-20 12:18:06 +02:00

Author

I figured, Blender seems to have multiple issues on Mac that I don't get on my pc at home.

I tried recreating it from a new file with Susans but have not succeeded. When I append the mesh from my project file into a new .blend the issue is visible though.

Thanks for taking the time.

I figured, Blender seems to have multiple issues on Mac that I don't get on my pc at home. I tried recreating it from a new file with Susans but have not succeeded. When I append the mesh from my project file into a new .blend the issue is visible though. Thanks for taking the time.

Alaska changed title from ~~uv islands display issue~~ to UV islands display issue - macOS Metal

2024-08-21 11:59:25 +02:00

Clément Foucault added the

label 2024-08-23 09:57:24 +02:00

Clément Foucault self-assigned this 2024-09-05 13:15:11 +02:00

Clément Foucault commented

2024-09-09 16:24:35 +02:00

Member

The root cause of the problem is that the Metal vertex fetch path does not consider the base index from the original render command. And this is used when the index buffer range of vertices is less than SHORT_MAX and not starting from 0.

The new Primitive Expansion API (which goal is to replace the Metal vertex fetch path) does the proper thing and is supposed to be used for UV drawing for the Overlay Next project #102179.

I will not take time to fix the Metal Vertex Fetch path and focus on the new implementation which should be default at the start of 4.4. However, it would be nice to fix for 4.2 and 4.3. So I will leave that to Apple to fix it.

The root cause of the problem is that the Metal vertex fetch path does not consider the base index from the original render command. And this is used when the index buffer range of vertices is less than SHORT_MAX and not starting from 0. The new Primitive Expansion API (which goal is to replace the Metal vertex fetch path) does the proper thing and is supposed to be used for UV drawing for the Overlay Next project #102179. I will not take time to fix the Metal Vertex Fetch path and focus on the new implementation which should be default at the start of 4.4. However, it would be nice to fix for 4.2 and 4.3. So I will leave that to Apple to fix it.

Clément Foucault added

and removed

labels 2024-09-09 17:11:51 +02:00

Clément Foucault commented

2024-09-16 12:49:53 +02:00

Member

@James-McCarthy-4 This is the issue I was mentioning in the meeting today. I am not sure how hard is it to fix for 4.2 / 4.3.

But it will definitely be fixed in 4.4 after replacing the Overlay engine by Overlay next.

@James-McCarthy-4 This is the issue I was mentioning in the meeting today. I am not sure how hard is it to fix for 4.2 / 4.3. But it will definitely be fixed in 4.4 after replacing the Overlay engine by Overlay next.

Clément Foucault removed their assignment 2024-09-16 12:50:06 +02:00

James-McCarthy-4 commented

2024-09-16 13:27:10 +02:00

Member

@fclem Just to be clear when you say 'base-index' do you mean the indexBufferOffset in the draw command? And you're saying it's not being taken into account when you read the vertex_id in the shader?

@fclem Just to be clear when you say 'base-index' do you mean the _indexBufferOffset_ in the draw command? And you're saying it's not being taken into account when you read the [[ vertex_id ]] in the shader?

Clément Foucault commented

2024-09-16 14:05:43 +02:00

Member

@James-McCarthy-4 Yes it is indexBufferOffset but in vertex, not in bytes. See gpu::IndexBuf::index_start_.

It is not taken into account inside the RESOLVE_VERTEX macro of the vertex fetch API.

#  define RESOLVE_VERTEX(v_id) \
    ((UNIFORM_SSBO_USES_INDEXED_RENDERING_STR > 0) ? \
         ((UNIFORM_SSBO_INDEX_MODE_U16_STR > 0) ? MTL_INDEX_DATA_U16[v_id] : \
                                                  MTL_INDEX_DATA_U32[v_id]) : \
         v_id)

It should be MTL_INDEX_DATA_U16[v_id] + index_start_. I guess it just needs to be passed as uniform.

@James-McCarthy-4 Yes it is `indexBufferOffset` but in vertex, not in bytes. See `gpu::IndexBuf::index_start_`. It is not taken into account inside the RESOLVE_VERTEX macro of the vertex fetch API. ``` # define RESOLVE_VERTEX(v_id) \ ((UNIFORM_SSBO_USES_INDEXED_RENDERING_STR > 0) ? \ ((UNIFORM_SSBO_INDEX_MODE_U16_STR > 0) ? MTL_INDEX_DATA_U16[v_id] : \ MTL_INDEX_DATA_U32[v_id]) : \ v_id) ``` It should be `MTL_INDEX_DATA_U16[v_id] + index_start_`. I guess it just needs to be passed as uniform.

James-McCarthy-4 commented

2024-09-16 17:15:20 +02:00

Member

@fclem Hi Clement, indexBufferOffset is just to used to shift the base of the provided index buffer. It's not something that should be added on to an index, the HW uses it when calculating the value of vertex_id to pass into a shader.

i.e. For 32bit indicies.
vertex_id = index_buffer[ vertex_index + (indexBaseOffset / 4)]

So possibly the issue lies somewhere else - unless I'm missing something.

@fclem Hi Clement, indexBufferOffset is just to used to shift the base of the provided index buffer. It's not something that should be added on to an index, the HW uses it when calculating the value of vertex_id to pass into a shader. i.e. For 32bit indicies. vertex_id = index_buffer[ vertex_index + (indexBaseOffset / 4)] So possibly the issue lies somewhere else - unless I'm missing something.

James-McCarthy-4 commented

2024-09-16 17:23:31 +02:00

Member

@fclem IIRC the behaviour for GL and Metal differs for values given for baseVertex maybe that's the issue?

@fclem IIRC the behaviour for GL and Metal differs for values given for _baseVertex_ maybe that's the issue?

Clément Foucault commented

2024-09-16 17:42:24 +02:00

Member

@James-McCarthy-4 My bad. You are right. It is in fact index_base_ (or baseVertex) that should be added to the index value from the index buffer.

I believe the indexBufferOffset is already taken into account when binding the IBO as Storage buffer.

I am pretty sure the behavior of GL and Metal regarding baseVertex is exactly the same since we feed the same value for MTLDrawList::append and GLDrawList::append. But the Metal Vertex Fetch API does not honor this specific offset (which IIRC is only used for 16bit indices).

@James-McCarthy-4 My bad. You are right. It is in fact `index_base_` (or `baseVertex`) that should be added to the index value from the index buffer. I believe the `indexBufferOffset` is already taken into account when binding the IBO as Storage buffer. I am pretty sure the behavior of GL and Metal regarding `baseVertex` is exactly the same since we feed the same value for `MTLDrawList::append` and `GLDrawList::append`. But the Metal Vertex Fetch API does not honor this specific offset (which IIRC is only used for 16bit indices).

James-McCarthy-4 commented

2024-09-16 19:42:57 +02:00

Member

@fclem You're correct - I was thinking of gl_baseVertex which has no equivalent in Metal but I don't think that has any relevance here.

" But the Metal Vertex Fetch API does not honor this specific offset (which IIRC is only used for 16bit indices)."
vertex_id does include the baseVertex according to the Metal Language spec:
vertex_id - ushort or uint - The per-vertex identifier, which includes the base vertex value if one is specified.

(There's a good summary here of the differences between the APIs here: https://github.com/gpuweb/gpuweb/issues/901)

I'm still not sure if we're looking at a compiler/drive bug or a Blender bug. I'll have a look through the Metal draw code.

@fclem You're correct - I was thinking of gl_baseVertex which has no equivalent in Metal but I don't think that has any relevance here. " But the Metal Vertex Fetch API does not honor this specific offset (which IIRC is only used for 16bit indices)." vertex_id does include the baseVertex according to the Metal Language spec: _vertex_id - ushort or uint - The per-vertex identifier, which includes the base vertex value if one is specified._ (There's a good summary here of the differences between the APIs here: https://github.com/gpuweb/gpuweb/issues/901) I'm still not sure if we're looking at a compiler/drive bug or a Blender bug. I'll have a look through the Metal draw code.

Clément Foucault commented

2024-09-17 11:33:52 +02:00

Member

@James-McCarthy-4

I am looking at a Blender Metal Backend bug (so in our codebase). The SSBO Vertex Fetch API was introduced by Michael PW to be able to workaround Metal's lack of geometry shader.

See the part after /* SSBO Vertex Fetch Mode. */ inside source/blender/gpu/shaders/metal/mtl_shader_defines.msl.

When this API is used (e.g. inside overlay_edit_uv_edges_vert_no_geom.glsl), the vertex_id does not represent the indices inside index buffer as the drawcall just emits X number of primitive (the max number of primitive you would expect after geometry shader expansion). The shader then use vertex_fetch_attribute function to load the index buffer value for a specific input vertex using manual IBO indirection. This indirection is the source of the bug.

You can find all code (that I found) related to this API in this PR #127660.

@James-McCarthy-4 I am looking at a Blender Metal Backend bug (so in our codebase). The SSBO Vertex Fetch API was introduced by Michael PW to be able to workaround Metal's lack of geometry shader. See the part after `/* SSBO Vertex Fetch Mode. */` inside `source/blender/gpu/shaders/metal/mtl_shader_defines.msl`. When this API is used (e.g. inside `overlay_edit_uv_edges_vert_no_geom.glsl`), the `vertex_id` does not represent the indices inside index buffer as the drawcall just emits X number of primitive (the max number of primitive you would expect after geometry shader expansion). The shader then use `vertex_fetch_attribute` function to load the index buffer value for a specific input vertex using manual IBO indirection. This indirection is the source of the bug. You can find all code (that I found) related to this API in this PR #127660.

Jason Fielder referenced a pull request that will close this issue

2024-09-19 16:13:47 +02:00

Fix #126542 Fix UV Edge overlays in Metal #127864

Clément Foucault referenced this issue from a commit

2024-09-25 15:22:47 +02:00

Fix #126542 Fix UV Edge overlays in Metal

Clément Foucault closed this issue

2024-09-25 15:22:49 +02:00

Blender Bot added

and removed

labels 2024-09-25 15:22:50 +02:00

Clément Foucault referenced this issue

2024-09-25 15:23:51 +02:00

Blender LTS: Maintenance Task 4.2 #124452

Philipp Oeser referenced this issue from a commit

2024-10-10 14:20:38 +02:00

Fix #126542 Fix UV Edge overlays in Metal

Sign in to join this conversation.

No Label

Animation & Rigging

Automated Testing

Blender Asset Bundle

Code Documentation

Dependency Graph

Development Management

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Viewport & EEVEE

Virtual Reality

Asset Browser Project

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Viewport & EEVEE

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#126542

No description provided.