Page MenuHome

UI: Transform Arrow Cursor Refactor
ClosedPublic

Authored by Harley Acheson (harley) on Nov 30 2020, 4:11 AM.

Details

Summary

When using the transform tools you quite often see some arrows used in place of the mouse cursor. This refactors the code that makes those arrows, mostly to address problems related to using the interface when using thin or thick lines. There is an approximate 36% reduction in code 344 lines down to 221.

The dashed line changes spacing with changes to line width to avoid getting thick and short dashes. The arrows themselves scale a bit nicer with changes in line width. And the arrowheads line up nicer. Code is removed because there is no need to draw arrow shafts as multiple curved segments. Without that need we don't have to draw arcs. And we don't need two arrow-drawing routines. And I also fixed a hack that assumed the shader position variable was always zero.

The following capture compares before and after this patch, admittedly at an extreme size at 4X scale. The top shows how it looks now when using thin, default, and thick lines. The bottom show the same after this patch is applied. The bottom set follows the increase in line width a bit nicer and closes the arrows nicely.

The following shows how it currently looks, with scale set to 2X and "wide lines". You will see that the arrows get too fat to look like arrows. Their points don't line up to a point. And they "blow up" and do odd things as you cross the center location:

This shows what is looks after this patch is applied with the same settings. The arrow dimensions are scaled to remain arrow-like. The points are sharp. And they behave correctly as they pass through the center:

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Nov 30 2020, 4:11 AM
Harley Acheson (harley) created this revision.
Hans Goudey (HooglyBoogly) requested changes to this revision.Dec 12 2020, 12:48 AM

I won't pretend to understand the need for a variable like "nudge", but I trust your work here anyway.

Overall the cursors look much better. I tested at different line widths and UI scales and they seem to work well. I just found one combination that I think looks worse:

BeforeAfter

I think the weird placement is just because I was trying to take a screenshot of the cursor

Hopefully it's possible to address that? It looks a bit "squat" with the patch applied.

This revision now requires changes to proceed.Dec 12 2020, 12:48 AM
Harley Acheson (harley) planned changes to this revision.Dec 12 2020, 1:02 AM

@Hans Goudey (HooglyBoogly) - I won't pretend to understand the need for a variable like "nudge", but I trust your work here anyway.

Nah, I'll find a better variable name or make that more obvious.

I just found one combination that I think looks worse:

Yes, I see that too. Should be able to figure that out.

Refactor of this file. From 344 to 236 lines, 31% reduction.

Arrow shafts are too short to be curved, and making them a single segment means we don't need to draw arcs nor do we need two arrow-drawing routines, or pass around arrow dimensions. Removing hack that assumed that shader_pos is zero. HLP_ANGLE no longer blows up when near center location.

Harley Acheson (harley) retitled this revision from UI: Transform Arrow Cursors to UI: Transform Arrow Cursor Refactor.Dec 12 2020, 9:26 PM
Harley Acheson (harley) edited the summary of this revision. (Show Details)
Harley Acheson (harley) edited the summary of this revision. (Show Details)Dec 12 2020, 9:29 PM
Hans Goudey (HooglyBoogly) accepted this revision.EditedDec 12 2020, 9:45 PM

This looks better in all combinations, even with settings where the cursors looked completely broken before.

I also think the way the current version (in master) tries to use the tangents of a sphere is pointless at best, and ugly and weird at worst.

Without looking into it in complete detail, the refactors seem to make sense too.
It looks like there are some other cases in t->helpline that aren't pictured here. Just to make sure, you tested those too, right?
Looks like the cursor type has some indirection from the corresponding tool with MouseInputMode, so it's a bit annoying to see where those cases are actually used.

This revision is now accepted and ready to land.Dec 12 2020, 9:45 PM
Harley Acheson (harley) planned changes to this revision.Dec 12 2020, 11:47 PM

This patch, and current code, do not work correctly on many Macs (that don't do gl lines wider than one). Having trouble with new polyline (AA) shaders though - unpredictable width and other oddness

Should now work on Macs that don't do wide gl lines.

This revision is now accepted and ready to land.Dec 13 2020, 12:58 AM

Fix to matrix pushes and pops.

@Hans Goudey (HooglyBoogly) - other cases in t->helpline that aren't pictured here. Just to make sure, you tested those too, right?

Yes, these are all the same arrows oriented in bunch of different ways. Most are pretty boring.

Only minor changes requested, fine to commit once these are done.

source/blender/editors/transform/transform_draw_cursors.c
52

Convention used elsewhere is pos / pos_id (prefer pos_id since it's less ambiguous).

137

Can call this pos_id too.

218

This isn't needed anymore (it was an old convention, now noted in function comment rB41a945d7469f: Docs: add note on convention for setting line-width).

Updating to incorporate changes suggested by @Campbell Barton (campbellbarton)

This revision was automatically updated to reflect the committed changes.