Fix T88167: Regression: no tooltip for syringe/picker, during picking object
ClosedPublic
Actions

Authored by Pratik Borhade (PratikPB2123) on May 11 2021, 12:45 PM.

Details

Reviewers

Jeroen Bakker (jbakker)
Julian Eisel (Severin)

Maniphest Tasks

T88167: Regression: no tooltip for syringe/picker, during picking object

Commits

rB9e6c4be731a5: Fix T88167: Regression: no tooltip for syringe/picker, during picking object

Summary

Fix T88167.

Caused by rB97defd9cd79b: Cryptomatte: Show Name of Object/Material Under The Cursor.

(BLI_rcti_isect_pt used here to confirm if cursor position is in between active region boundary.
Subtracting min region boundary from the mouse position before the check, fails the condition.

mval[2] introduced to hold the region relative mouse position.

Diff Detail

Repository: rB Blender

Event Timeline

Pratik Borhade (PratikPB2123) requested review of this revision.May 11 2021, 12:45 PM

Pratik Borhade (PratikPB2123) created this revision.

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)May 11 2021, 12:48 PM

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)

Pratik Borhade (PratikPB2123) added reviewers: Jeroen Bakker (jbakker), Julian Eisel (Severin).

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)May 11 2021, 12:50 PM

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)May 11 2021, 12:53 PM

I might have missed that when reshuffling the code. LGTM!

I personally don't like it if variables change semantics (like here: starting as screen relative, then becoming region relative).
I'd suggest having a const int mval[2] (which by convention is our name for region relative mouse coordinates), and either pass win->eventstate->x/y directly to the intersection check, or have a separate const int variable for that.
Note that there also is win->eventstate->mval for region relative event coordinates, but that may not actually be set during drawing, not sure.

This revision now requires changes to proceed.May 11 2021, 9:36 PM

Vyacheslav (hitrpr) awarded a token.May 12 2021, 12:00 AM

mval[2] introduced to hold region relative mouse position.

(win->eventstate->mval is not updating so followed original logic)

Looks fine, but still suggesting a change.

source/blender/editors/interface/interface_eyedropper.c
140–146	Why not this?: const int mval[2] = { x - region->winrct.xmin, y - region->winrct.ymin, }; eyedropper_draw_cursor_text_ex(mval[0], mval[1], name); We usually prefer keeping declarations close to usage.