Fix #107411: Non-visible objects in the viewport occlude selection #107449

Closed

Guillermo Venegas wants to merge 1 commits from guishe:local-view-selection into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

Guillermo Venegas commented

2023-04-28 18:48:50 +02:00

Contributor

When using edit mode with a mesh, if you have retopology overlay active,
the selection of vertices, edges, or faces is occluded by the other objects.
However, if local view is on or collections with objects are hidden,
non-visible objects can occlude the selection.

This patch discards objects that are not visible in the viewport so they do not
occlude the selection anymore.

When using edit mode with a mesh, if you have retopology overlay active, the selection of vertices, edges, or faces is occluded by the other objects. However, if local view is on or collections with objects are hidden, non-visible objects can occlude the selection. This patch discards objects that are not visible in the viewport so they do not occlude the selection anymore.

Guillermo Venegas force-pushed local-view-selection from a6ef7f443a to 710e4a228a

2023-04-28 18:53:06 +02:00

Compare

Guillermo Venegas changed title from ~~Fix #107411: Selection occlusion in local view~~ to Fix #107411: Selection occlusion in local view view with retopology overlay

2023-04-28 18:53:07 +02:00

Guillermo Venegas changed title from ~~Fix #107411: Selection occlusion in local view view with retopology overlay~~ to Fix #107411: Selection occlusion in local view with retopology overlay

2023-04-28 18:53:15 +02:00

Iliya Katushenock added this to the EEVEE & Viewport project 2023-04-28 19:17:21 +02:00

Iliya Katushenock added the

label 2023-04-28 19:17:25 +02:00

Jorijn de Graaf commented

2023-05-15 17:55:20 +02:00

Contributor

I'd do a slightly different check:

if (!BKE_object_is_visible_in_viewport(v3d, ob)) {
    continue;
}

Perhaps before the other checks, to be consistent with other code in the same file.
And then I'd rename this to something like "Objects hidden by local mode or collection occlude selection", because it's not just local mode that's fixed.

I'd do a slightly different check: ```cpp if (!BKE_object_is_visible_in_viewport(v3d, ob)) { continue; } ``` Perhaps before the other checks, to be consistent with other code in the same file. And then I'd rename this to something like "Objects hidden by local mode or collection occlude selection", because it's not just local mode that's fixed.

Guillermo Venegas force-pushed local-view-selection from 710e4a228a to b9f9fc08b4

2023-05-15 18:15:24 +02:00

Compare

Guillermo Venegas force-pushed local-view-selection from b9f9fc08b4 to 44dfb9666e

2023-05-15 18:26:09 +02:00

Compare

Guillermo Venegas changed title from ~~Fix #107411: Selection occlusion in local view with retopology overlay~~ to Fix #107411: Non-visible objects in the viewport affect selection collusion

2023-05-15 18:26:38 +02:00

Guillermo Venegas changed title from ~~Fix #107411: Non-visible objects in the viewport affect selection collusion~~ to Fix #107411: Non-visible objects in the viewport occlude selection

2023-05-15 19:03:06 +02:00

Thomas Dinges commented

2023-05-15 20:52:50 +02:00

Owner

@fclem @Jeroen-Bakker Hey, can you please take a look at this PR? Thanks in advance.

@fclem @Jeroen-Bakker Hey, can you please take a look at this PR? Thanks in advance.

Jeroen Bakker commented

2023-05-15 21:27:47 +02:00

Member

We have two PRs to fix the same issue #107833. Note that this should be solved in the engines and not in the draw manager itself. the draw manager should not have this logic if not needed.

We have two PRs to fix the same issue #107833. Note that this should be solved in the engines and not in the draw manager itself. the draw manager should not have this logic if not needed.

Jorijn de Graaf commented

2023-05-15 21:39:25 +02:00

Contributor

This same check is done in 3 other depsgraph iterators in this same file, so I think it makes sense here.
But moving it to the select engine cache populate function is fine too I suppose.
Like so:

static void select_cache_populate(void *vedata, Object *ob)
{
  SELECTID_StorageList *stl = ((SELECTID_Data *)vedata)->stl;
  const DRWContextState *draw_ctx = DRW_context_state_get();

  const bool retopology_occlusion = RETOPOLOGY_ENABLED(draw_ctx->v3d) &&
                                    !XRAY_ENABLED(draw_ctx->v3d);
  if (retopology_occlusion && !DRW_object_is_in_edit_mode(ob)) {
-   if (ob->dt >= OB_SOLID) {
+   if (ob->dt >= OB_SOLID && BKE_object_is_visible_in_viewport(draw_ctx->v3d, ob)) {
      struct GPUBatch *geom_faces = DRW_mesh_batch_cache_get_surface(ob->data);
      DRW_shgroup_call_obmat(stl->g_data->shgrp_occlude, geom_faces, ob->object_to_world);
    }
    return;
  }

...

This same check is done in 3 other depsgraph iterators in this same file, so I think it makes sense here. But moving it to the select engine cache populate function is fine too I suppose. Like so: ```diff static void select_cache_populate(void *vedata, Object *ob) { SELECTID_StorageList *stl = ((SELECTID_Data *)vedata)->stl; const DRWContextState *draw_ctx = DRW_context_state_get(); const bool retopology_occlusion = RETOPOLOGY_ENABLED(draw_ctx->v3d) && !XRAY_ENABLED(draw_ctx->v3d); if (retopology_occlusion && !DRW_object_is_in_edit_mode(ob)) { - if (ob->dt >= OB_SOLID) { + if (ob->dt >= OB_SOLID && BKE_object_is_visible_in_viewport(draw_ctx->v3d, ob)) { struct GPUBatch *geom_faces = DRW_mesh_batch_cache_get_surface(ob->data); DRW_shgroup_call_obmat(stl->g_data->shgrp_occlude, geom_faces, ob->object_to_world); } return; } ... ```

Guillermo Venegas force-pushed local-view-selection from 44dfb9666e to 93f43b5172

2023-05-16 04:04:17 +02:00

Compare

Guillermo Venegas force-pushed local-view-selection from 93f43b5172 to 75d72daba3

2023-05-16 04:13:18 +02:00

Compare

Jeroen Bakker commented

2023-05-16 14:20:12 +02:00

Member

Ah yes you're right. Sorry about that. Do you mind reverting it to the previous version. Than I can accept it

Ah yes you're right. Sorry about that. Do you mind reverting it to the previous version. Than I can accept it

Guillermo Venegas force-pushed local-view-selection from 75d72daba3 to e7c4544c84

2023-05-16 16:52:58 +02:00

Compare

Guillermo Venegas force-pushed local-view-selection from e7c4544c84 to 0d3914f646

2023-05-16 16:54:05 +02:00

Compare

Jorijn de Graaf commented

2023-05-18 23:03:42 +02:00

Contributor

@Jeroen-Bakker He's reverted it like you asked 🙂

@Jeroen-Bakker He's reverted it like you asked 🙂

Jeroen Bakker approved these changes 2023-05-22 08:21:56 +02:00

Jeroen Bakker commented

2023-05-22 08:28:24 +02:00

Member

I manually commited this to blender-v3.6-release branch e83a2386b5

I manually commited this to blender-v3.6-release branch e83a2386b5f6dcdb60b9c57934b5642da37853b1

Jeroen Bakker closed this pull request

2023-05-22 08:28:25 +02:00

Guillermo Venegas deleted branch local-view-selection

2023-05-22 16:55:52 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

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#107449

No description provided.