feat(iv): area probe

[dann1m]

Fixes #4714

Description

This PR adds support for selecting a rectangular region within an image to compute and display per-channel minimum, maximum, and average values.

The feature can be activated through the Tools menu or by using a keyboard shortcut (Ctrl-A / Cmd-A) . Once active, the user can click and drag with the left mouse button to define a rectangular area, which is visually indicated by a blue outline. Upon releasing the mouse button, the tool calculates statistics for all pixels within the selected area and displays the results in the status bar.

Checklist:

• I have read the contribution guidelines.
• I have updated the documentation, if applicable. (Check if there is no
  need to update the documentation, for example if this is a bug fix that
  doesn't change the API.)
• I have ensured that the change is tested somewhere in the testsuite
  (adding new test cases if necessary).
• If I added or modified a C++ API call, I have also amended the
  corresponding Python bindings (and if altering ImageBufAlgo functions, also
  exposed the new functionality as oiiotool options).
• My code follows the prevailing code style of this project. If I haven't
  already run clang-format before submitting, I definitely will look at the CI
  test that runs clang-format and fix anything that it highlights as being
  nonconforming.

[lgritz]

Hi, Danielle, it's great to see you here, and this is a good start!

I tried this out on my end, and I have a number of comments. I'm just going to list them all, in no particular order. Some of this is going to be pretty picky about minor UI issues. I think they are all fairly minor in the grand scheme of things, so don't be discouraged that it's not 100% on the first try. (Nothing I do with UIs ever is.)

• Is there a reason for 'E' as the hotkey? Does it stand for something?
• When I hit 'E', there is no immediate visual feedback that I'm in "sweep an area to sample" mode. I think that perhaps if the cursor changes at that point to a different shape (especially if it somehow indicates or invites the user is expected to sweep out an area), it would be helpful. Or, maybe it's enough to change the status bar (or display the overlay) when 'E' is hit, instead of only after the area is swept? Perhaps that would make it more visually obvious that you're in a new mode.
• After the sweep, the display changes in a fairly jarring way:
  • The status bar at the bottom goes from being strictly one line, to being vertically thick with each channel taking an additional line. This isn't necessarily bad by itself, though we should consider whether it "scales", for example, is it going to look really bad if you're viewing an image with 27 channels? This may be turn out to be the best argument for showing the area info in an overlay, like the 'P' pixel mode display, instead of on the status bar.
  • The viewed image itself jumps to a different position. I think this is a real problem. We definitely want the displayed image to stay in the previous spot, even if the size of the status line changes.
• The channels are listed as "ch0", "ch1". We should use the real names, like the 'P' pixel view overlay does.
• Not a problem: I like the fact that you can actually enable the pixel view mode and the area sweep mode simultaneously!
• When you hit 'E' again to go back to the usual mode again, the image pops position again, and also changes color (!) until you move the mouse again, and then goes back to normal. Not sure what that's all about.

[dann1m]

@lgritz, I made the suggested changes to the feature if you would like to take a look at it.
There is an issue with one of the commits not recognizing the signature, so I attempted rebasing a couple of times, and also a different pr entirely, but to no avail.

[lgritz]

I think the problem is that you tried to "merge". That's rarely necessary for a PR unless you're pretty sure there is a direct conflict between what you're working on and something committed since you branched off. I think that doing a "rebase" on top of the new main is much better, and yields a simple linear history.

Hang on, I think I can fix this for you... Do you mind if I push a fix to your branch?

[lgritz]

By the way, it's good to see you back here, Danielle! I was really hoping you were going to pick up this work again and be able to get it fully merged.

[dann1m]

I don't mind at all, thank you so much! Sorry, I took time with it; school was winding down. It's good to be back!

[lgritz]

ok, I pushed the changes to your fork -- cleanly rebased on top of main, fixed the one broken signature that was holding up the DCO.

You'll need to fetch or pull on your end to get my changes to your local machine, and move your branch marker to the new one. (Or do a fresh checkout.)

[dann1m]

Thank you @lgritz! I did a bit of cleaning, so everything should work now. When you have time, I would appreciate feedback

[lgritz]

I pulled this to my machine, did a build, and played around with it a bit. Behavior-wise, this looks really good to me! I like how it works. It's also handy that when you're in area mode, if you just click, it shows you the value of that pixel. I don't have any additional complaints or suggestions about the behavior at this time. I'll take one more look at the code itself and see if I have any suggestions there.

[lgritz]

The code totally looks great. I made some incredibly minor suggestions, I think all about whitespace (we like 3 blank lines between standalone functions, and at most 1 consecutive blank line within a function body), and pointed out some areas of commented-out code that I suspect are code you were trying out at one point but but now can be removed from the final commit.

That's it. Quickly fix those things up, and we are definitely ready to merge!

This is a cool new feature, Danielle. Thanks for the great work.

[dann1m]

No problem! I'll deal with those now

[lgritz]

LGTM, Danielle. Thanks for making the corrections quickly. Looks great, congrats!