Page MenuHome

Eyedropper: Support sample other windows
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Jun 2 2020, 6:59 PM.

Details

Summary

This is a proposal to fix T77226

I had to create a set of new functions to support this.
Which looks bad at first as WM_api.h already has many functions that
don't look like utilities.

Also, I'm not sure if the names "_desktop_pos" and "_screen_pos"
are correct for this case.

Ref T77226

Diff Detail

Repository
rB Blender

Event Timeline

Germano Cavalcante (mano-wii) requested review of this revision.Jun 2 2020, 6:59 PM
Brecht Van Lommel (brecht) requested changes to this revision.Jun 2 2020, 9:01 PM

Nice to see this finally tackled.

source/blender/editors/interface/interface_eyedropper_color.c
197–206

Can this logic be moved earlier, so that it will also use specific code for the image/node/clip editor in other windows?

source/blender/windowmanager/WM_api.h
109–112

Maybe these two API functions could be changed into one WM_window_find_under_cursor()?

source/blender/windowmanager/intern/wm_window.c
2012–2022

Does this correctly handle overlapping windows? Seems like it would need to check the order somehow?

The window might also be minimized, probably a test for that is needed?

I guess the existing code this was copied from already had the problem, if there is any.

2041

Is adding glFinish() really needed?

This revision now requires changes to proceed.Jun 2 2020, 9:01 PM
Germano Cavalcante (mano-wii) marked 2 inline comments as done.
  • Unify WM funcition and anticipate the logic to get the screen and the area under the cursor
source/blender/windowmanager/intern/wm_window.c
2041

Probably not, but I copied this from the function below WM_window_pixels_read.
So in doubt ... leave it like this?

Germano Cavalcante (mano-wii) planned changes to this revision.Jun 2 2020, 9:54 PM

I did something wrong because it's not working anymore!

source/blender/windowmanager/intern/wm_window.c
2012–2022

This is indeed a problem.
But I fear that the information of the order of the windows is something internal in Ghost.
This could easily be solved if we always move the active window to the beginning of the list.

  • Ignore minimized windows
  • Add win_ignore variable to avoid breakage in the event system
  • Missed in previous patch
This revision is now accepted and ready to land.Jun 3 2020, 7:29 PM
source/blender/windowmanager/intern/wm_window.c
2012–2022

There could be an arbitrary number of windows stacked.

I think ideally we'd add some GHOST API function to get the order number of a window. But this can be left as a future improvement.

Please leave a comment in the code about that.

Germano Cavalcante (mano-wii) marked 3 inline comments as done.
  • Remove glFinish (the original code does not use this resourse and we can re-implement if there are problems)
  • Comment limitation in getting the order of windows

I can't seem to get this to work on Ubuntu 20.04, with a dual monitor setup and two Main windows of the same blender instance.

Also, is this intended to work with all the dropper tool or just for the colour picker? I asked this related question on stackexchange almost two years ago.

Can't also make it work on Pop!OS 20.04.

Should we make a separate bug report for this?

Should we make a separate bug report for this?

Oops. Yes, please, this is recommended.
Although I won't be able to fix it, since I can't reproduce the problem.
But I have some suspicions of what may be causing this.
Thanks.