Popovers scroll arrows glitch #65351

Closed
opened 4 years ago by jenkm · 17 comments
jenkm commented 4 years ago

System Information
Operating system: Darwin-18.2.0-x86_64-i386-64bit 64 Bits
Graphics card: AMD Radeon Pro 455 OpenGL Engine ATI Technologies Inc. 4.1 ATI-2.4.10

Blender Version
Broken: version: 2.80 (sub 72), branch: blender2.7, commit date: 2019-05-29 18:17, hash: fee600f479

Short description of error

When you scroll popovers, the content goes beyond the boundaries:

1.jpg

2.jpg

The active area of scroll arrows that responds to mouse hover is shifted:

3.jpg


There is an offset for backdrop, in ui_draw_popover_back_impl, but not for the content itself, this causes this error.

  rect->ymax -= unit_half;
  rect->ymin += unit_half;
**System Information** Operating system: Darwin-18.2.0-x86_64-i386-64bit 64 Bits Graphics card: AMD Radeon Pro 455 OpenGL Engine ATI Technologies Inc. 4.1 ATI-2.4.10 **Blender Version** Broken: version: 2.80 (sub 72), branch: blender2.7, commit date: 2019-05-29 18:17, hash: `fee600f479` **Short description of error** When you scroll popovers, the content goes beyond the boundaries: ![1.jpg](https://archive.blender.org/developer/F7081234/1.jpg) ![2.jpg](https://archive.blender.org/developer/F7081235/2.jpg) The active area of scroll arrows that responds to mouse hover is shifted: ![3.jpg](https://archive.blender.org/developer/F7081236/3.jpg) *** There is an offset for backdrop, in `ui_draw_popover_back_impl`, but not for the content itself, this causes this error. ``` rect->ymax -= unit_half; rect->ymin += unit_half; ```
jenkm commented 4 years ago
Poster

Added subscriber: @jenkm

Added subscriber: @jenkm
jenkm commented 4 years ago
Poster

Added subscriber: @WilliamReynish

Added subscriber: @WilliamReynish
jenkm commented 4 years ago
Poster

This comment was removed by @jenkm

*This comment was removed by @jenkm*
jenkm commented 3 years ago
Poster

Removed subscriber: @WilliamReynish

Removed subscriber: @WilliamReynish
jenkm commented 3 years ago
Poster

There is an offset for backdrop, in ui_draw_popover_back_impl, but not for the content itself, this causes this error.

  rect->ymax -= unit_half;
  rect->ymin += unit_half;
There is an offset for backdrop, in `ui_draw_popover_back_impl`, but not for the content itself, this causes this error. ``` rect->ymax -= unit_half; rect->ymin += unit_half; ```
iss commented 3 years ago
Collaborator

Added subscriber: @iss

Added subscriber: @iss
iss commented 3 years ago
Collaborator

Changed status from 'Confirmed' to: 'Needs User Info'

Changed status from 'Confirmed' to: 'Needs User Info'
iss commented 3 years ago
Collaborator

I have re-triaged this report and can not reproduce it.
Can you please check if this is still an issue in latest build?
https://builder.blender.org/download/

I have re-triaged this report and can not reproduce it. Can you please check if this is still an issue in latest build? https://builder.blender.org/download/
jenkm commented 3 years ago
Poster

Added subscriber: @brecht

Added subscriber: @brecht
jenkm commented 3 years ago
Poster

It's become less noticeable after the D6483, but it's still here.

popover-offset.png

You can just check where the on mouse hover scrolling active zone is, and see it is shifted relative to the arrow that marks it.

3.jpg

Anyway, I have a patch for that. I think @brecht can take a look at this.
Maybe there's a more elegant solution.

diff --git a/source/blender/editors/interface/interface_region_popup.c b/source/blender/editors/interface/interface_region_popup.c
index 867ac652505..56652bf170a 100644
--- a/source/blender/editors/interface/interface_region_popup.c
+++ b/source/blender/editors/interface/interface_region_popup.c
@@ -708,6 +708,15 @@ uiBlock *ui_popup_block_refresh(bContext *C,
     }
   }
   else {
+    /* Add an offset to draw the popover arrow. */
+    if ((block->flag & UI_BLOCK_POPOVER) && ELEM(block->direction, UI_DIR_UP, UI_DIR_DOWN)) {
+      /* Keep sync with the 'ui_draw_popover_back'. */
+      const float unit_size = U.widget_unit / block->aspect;
+      const float unit_half = unit_size * (block->direction == UI_DIR_DOWN ? 0.5 : -0.5);
+
+      UI_block_translate(block, 0, -unit_half);
+    }
+
     /* clip block with window boundary */
     ui_popup_block_clip(window, block);
 
diff --git a/source/blender/editors/interface/interface_widgets.c b/source/blender/editors/interface/interface_widgets.c
index 92032c3b18c..8c28b29f5ab 100644
--- a/source/blender/editors/interface/interface_widgets.c
+++ b/source/blender/editors/interface/interface_widgets.c
@@ -4970,8 +4970,6 @@ static void ui_draw_popover_back_impl(const uiWidgetColors *wcol,
                                              rect->xmin + unit_size,
                                              rect->xmax - unit_size) :
                                      BLI_rcti_cent_x(rect);
-  rect->ymax -= unit_half;
-  rect->ymin += unit_half;
 
   GPU_blend(true);
 
It's become less noticeable after the [D6483](https://archive.blender.org/developer/D6483), but it's still here. ![popover-offset.png](https://archive.blender.org/developer/F8320760/popover-offset.png) You can just check where the on mouse hover scrolling active zone is, and see it is shifted relative to the arrow that marks it. ![3.jpg](https://archive.blender.org/developer/F7081236/3.jpg) Anyway, I have a patch for that. I think @brecht can take a look at this. Maybe there's a more elegant solution. ``` diff --git a/source/blender/editors/interface/interface_region_popup.c b/source/blender/editors/interface/interface_region_popup.c index 867ac652505..56652bf170a 100644 --- a/source/blender/editors/interface/interface_region_popup.c +++ b/source/blender/editors/interface/interface_region_popup.c @@ -708,6 +708,15 @@ uiBlock *ui_popup_block_refresh(bContext *C, } } else { + /* Add an offset to draw the popover arrow. */ + if ((block->flag & UI_BLOCK_POPOVER) && ELEM(block->direction, UI_DIR_UP, UI_DIR_DOWN)) { + /* Keep sync with the 'ui_draw_popover_back'. */ + const float unit_size = U.widget_unit / block->aspect; + const float unit_half = unit_size * (block->direction == UI_DIR_DOWN ? 0.5 : -0.5); + + UI_block_translate(block, 0, -unit_half); + } + /* clip block with window boundary */ ui_popup_block_clip(window, block); diff --git a/source/blender/editors/interface/interface_widgets.c b/source/blender/editors/interface/interface_widgets.c index 92032c3b18c..8c28b29f5ab 100644 --- a/source/blender/editors/interface/interface_widgets.c +++ b/source/blender/editors/interface/interface_widgets.c @@ -4970,8 +4970,6 @@ static void ui_draw_popover_back_impl(const uiWidgetColors *wcol, rect->xmin + unit_size, rect->xmax - unit_size) : BLI_rcti_cent_x(rect); - rect->ymax -= unit_half; - rect->ymin += unit_half; GPU_blend(true); ```
iss commented 3 years ago
Collaborator

Changed status from 'Needs User Info' to: 'Confirmed'

Changed status from 'Needs User Info' to: 'Confirmed'
brecht commented 3 years ago
Owner

This patch adds spacing to the top over every popover, also without scrolling, and it doesn't look good.

I think somehow this should only be done when scrolling somehow?

The patch can also be simplified by using:

UI_block_translate(block, 0, -unit_half);
This patch adds spacing to the top over every popover, also without scrolling, and it doesn't look good. I think somehow this should only be done when scrolling somehow? The patch can also be simplified by using: ``` UI_block_translate(block, 0, -unit_half); ```
jenkm commented 3 years ago
Poster

There is another problem, when you scroll down the popover and then go back it doesn't return to the original position, the margin gets smaller.

Now (initially open vs scrolled):

popover-scroll-before.png

popover-scroll-after.png

After patch above (initially open vs scrolled):

popover-scroll-patch-before.png

popover-scroll-patch-after.png

There is another problem, when you scroll down the popover and then go back it doesn't return to the original position, the margin gets smaller. Now (initially open vs scrolled): ![popover-scroll-before.png](https://archive.blender.org/developer/F8341231/popover-scroll-before.png) ![popover-scroll-after.png](https://archive.blender.org/developer/F8341230/popover-scroll-after.png) After patch above (initially open vs scrolled): ![popover-scroll-patch-before.png](https://archive.blender.org/developer/F8342916/popover-scroll-patch-before.png) ![popover-scroll-patch-after.png](https://archive.blender.org/developer/F8342915/popover-scroll-patch-after.png)
jenkm commented 3 years ago
Poster

Ok, this extra offset comes from ui_menu_scroll_apply_offset_y.

popovers-offset-patch.patch

Ok, this extra offset comes from `ui_menu_scroll_apply_offset_y`. [popovers-offset-patch.patch](https://archive.blender.org/developer/F8343121/popovers-offset-patch.patch)
Collaborator

This issue was referenced by fe3ce61528

This issue was referenced by fe3ce615287ae8c6fa5694c03a07d8699d7c8aff
brecht commented 3 years ago
Owner

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'
brecht closed this issue 3 years ago
brecht self-assigned this 3 years ago
jenkm commented 3 years ago
Poster

@brecht there was a problem after this patch, the arrow is now cropped. Any ideas?

And it's only for down directional, the upward direction works fine.

{F8400498, size=full}

The upward direction works fine:

{F8400511, size=full}

@brecht there was a problem after this patch, the arrow is now cropped. Any ideas? And it's only for down directional, the upward direction works fine. {[F8400498](https://archive.blender.org/developer/F8400498/arrow-bug.png), size=full} The upward direction works fine: {[F8400511](https://archive.blender.org/developer/F8400511/arrow-bug-up.png), size=full}
Sign in to join this conversation.
No Label
Interest/Alembic
Interest/Animation & Rigging
Interest/Asset Browser
Interest/Asset Browser Project Overview
Interest/Audio
Interest/Automated Testing
Interest/Blender Asset Bundle
Interest/Collada
Interest/Compositing
Interest/Core
Interest/Cycles
Interest/Dependency Graph
Interest/Development Management
Interest/Eevee & Viewport
Interest/Freestyle
Interest/Geometry Nodes
Interest/Grease Pencil
Interest/ID Management
Interest/Images & Movies
Interest/Import/Export
Interest/Line Art
Interest/Masking
Interest/Modeling
Interest/Modifiers
Interest/Motion Tracking
Interest/Nodes & Physics
Interest/Overrides
Interest/Performance
Interest/Performance
Interest/Physics
Interest/Pipeline, Assets & I/O
Interest/Platforms, Builds, Tests & Devices
Interest/Python API
Interest/Render & Cycles
Interest/Render Pipeline
Interest/Sculpt, Paint & Texture
Interest/Text Editor
Interest/Translations
Interest/Triaging
Interest/Undo
Interest/USD
Interest/User Interface
Interest/UV Editing
Interest/VFX & Video
Interest/Video Sequencer
Interest/Virtual Reality
legacy module/Animation & Rigging
legacy module/Core
legacy module/Development Management
legacy module/Eevee & Viewport
legacy module/Grease Pencil
legacy module/Modeling
legacy module/Nodes & Physics
legacy module/Pipeline, Assets & IO
legacy module/Platforms, Builds, Tests & Devices
legacy module/Python API
legacy module/Rendering & Cycles
legacy module/Sculpt, Paint & Texture
legacy module/Triaging
legacy module/User Interface
legacy module/VFX & Video
legacy project/1.0.0-beta.2
legacy project/Asset Browser (Archived)
legacy project/BF Blender: 2.8
legacy project/BF Blender: After Release
legacy project/BF Blender: Next
legacy project/BF Blender: Regressions
legacy project/BF Blender: Unconfirmed
legacy project/Blender 2.70
legacy project/Code Quest
legacy project/Datablocks and Libraries
legacy project/Eevee
legacy project/Game Animation
legacy project/Game Audio
legacy project/Game Data Conversion
legacy project/Game Engine
legacy project/Game Logic
legacy project/Game Physics
legacy project/Game Python
legacy project/Game Rendering
legacy project/Game UI
legacy project/GPU / Viewport
legacy project/GSoC
legacy project/Infrastructure: Websites
legacy project/LibOverrides - Usability and UX
legacy project/Milestone 1: Basic, Local Asset Browser
legacy project/Nodes
legacy project/OpenGL Error
legacy project/Papercut
legacy project/Pose Library Basics
legacy project/Retrospective
legacy project/Tracker Curfew
legacy project/Wintab High Frequency
Meta/Good First Issue
Meta/Papercut
migration/requires-manual-verification
Module › Animation & Rigging
Module › Core
Module › Development Management
Module › Eevee & Viewport
Module › Grease Pencil
Module › Modeling
Module › Nodes & Physics
Module › Pipeline, Assets & IO
Module › Platforms, Builds, Tests & Devices
Module › Python API
Module › Render & Cycles
Module › Sculpt, Paint & Texture
Module › Triaging
Module › User Interface
Module › VFX & Video
Platform/FreeBSD
Platform/Linux
Platform/macOS
Platform/Windows
Priority › High
Priority › Low
Priority › Normal
Priority › Unbreak Now!
Status › Archived
Status › Confirmed
Status › Duplicate
Status › Needs Information from Developers
Status › Needs Information from User
Status › Needs Triage
Status › Resolved
Type › Bug
Type › Design
Type › Known Issue
Type › Patch
Type › Report
Type › To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#65351
Loading…
There is no content yet.