Improve WIA Cropping #4223
Improve WIA Cropping #4223
Conversation
adghayes
force-pushed
the
drew/write-in-view
branch
from
4c6c7ab to
ce22946
Compare
adghayes
force-pushed
the
drew/write-in-view
branch
from
142b28e to
52a40ca
Compare
adghayes
force-pushed
the
drew/write-in-view
branch
from
52a40ca to
5f4e3eb
Compare
| top: 2, | ||
| top: 1, |
There was a problem hiding this comment.
We were cropping a little high on the VxBallots, see difference between highlight and desired area:
The cropping too low is dealt with in the interpreter. The above is correct - it says 1 and we want 1. That off-by-one issue is in the interpreter.
There was a problem hiding this comment.
This is not necessary but thought I'd make it consistent.
| let width: GridUnit = grid_layout.option_bounds_from_target_mark.left | ||
| + grid_layout.option_bounds_from_target_mark.right | ||
| + 1; | ||
| + grid_layout.option_bounds_from_target_mark.right; | ||
| let height: GridUnit = grid_layout.option_bounds_from_target_mark.top | ||
| + grid_layout.option_bounds_from_target_mark.bottom | ||
| + 1; | ||
| + grid_layout.option_bounds_from_target_mark.bottom; |
There was a problem hiding this comment.
Here's the off-by-one issue. We're adding in 1 to the width and the height, in addition to the top + bottom or left + right specified by the gridLayouts. I imagine the original thought here was to account for the size of the bubble itself? But we don't need to do that, the measurements are from the center of the bubble in all directions.
There was a problem hiding this comment.
Weird, I don't know why we would have done that! Nice catch
| let width: GridUnit = grid_layout.option_bounds_from_target_mark.left | ||
| + grid_layout.option_bounds_from_target_mark.right | ||
| + 1; | ||
| + grid_layout.option_bounds_from_target_mark.right; | ||
| let height: GridUnit = grid_layout.option_bounds_from_target_mark.top | ||
| + grid_layout.option_bounds_from_target_mark.bottom | ||
| + 1; | ||
| + grid_layout.option_bounds_from_target_mark.bottom; |
There was a problem hiding this comment.
Weird, I don't know why we would have done that! Nice catch
Overview
#4025
Demo Video or Screenshot
Testing Plan
Manual mostly, updated snapshots / fixtures.
Checklist