Hang the auto-crop off the slider's commit, not off the pointer passing over
The straighten auto-crop worked once and then stopped. It was keyed on `PlainSlider::drag-changed`, whose name is a lie inherited from what it forwards: `SliderTrack` defines `engaged` as `has-hover || claimed`, and it has to — a `Flickable` withholds the press for 100ms, so hover is the only signal that arrives in time to stand the scrolling ancestor down. That is the right definition for the job it was written for and the wrong one for this. Keyed on hover, the correction fires when the pointer first crosses the track — before anything has been dragged — and then does not fire again for as long as the pointer stays on it, however many times the angle is changed. Which is exactly what "it only works once" looks like. `SliderTrack` already publishes the signal this wants. `committed` fires on release, once per gesture, after the final `changed`, and its doc comment says so in as many words. It was simply not forwarded through `PlainSlider`, so it now is, and the geometry panel's callback is a `committed(float)` rather than a `drag-changed(bool)`. The angle needs no re-applying here: the track emits its last `changed` before it commits, so the value is already in the graph by the time this runs. What is left is the correction that has to happen exactly once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+12
-4
@@ -2707,13 +2707,21 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// Only on release, and only on release. Doing it per frame ratchets
|
||||
// the crop down through every angle the slider passed through rather
|
||||
// than the one it stopped at; see `auto_crop_to_angle`.
|
||||
//
|
||||
// Hung off the track's `committed`, not off `engaged-changed`. The
|
||||
// latter is *hover* — it has to be, because a `Flickable` withholds
|
||||
// the press — so a correction keyed on it fires when the pointer first
|
||||
// crosses the track, whether or not anything was dragged, and then
|
||||
// never fires again for as long as the pointer stays on it. Which
|
||||
// reads, exactly, as an auto-crop that works once.
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
window.on_straighten_drag_changed(move |dragging| {
|
||||
if dragging {
|
||||
return;
|
||||
}
|
||||
window.on_straighten_committed(move |_degrees| {
|
||||
// The angle itself is already applied: `SliderTrack` emits its
|
||||
// last `changed` before it commits, so `on_straighten_changed` has
|
||||
// run with this very value. What is left is the correction that
|
||||
// has to happen exactly once.
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
s.auto_crop_to_angle();
|
||||
|
||||
@@ -324,6 +324,17 @@ component PlainSlider inherits Rectangle {
|
||||
in property <string> unit;
|
||||
|
||||
callback changed(float);
|
||||
/// The gesture is over and this is the value to keep.
|
||||
///
|
||||
/// **Not the same thing as `drag-changed`, which is hover.** `SliderTrack`
|
||||
/// defines `engaged` as `has-hover || claimed`, because a `Flickable`
|
||||
/// withholds the press and hover is the only signal that gets through in
|
||||
/// time to stand it down. That makes it exactly right for what it is for —
|
||||
/// telling a scrolling ancestor to let go — and exactly wrong for anything
|
||||
/// that must run once when the user has finished choosing: it fires on the
|
||||
/// way past without a drag at all, and then does not fire again while the
|
||||
/// pointer stays on the track, however many times the value is dragged.
|
||||
callback committed(float);
|
||||
callback reset();
|
||||
callback drag-changed(bool);
|
||||
|
||||
@@ -341,6 +352,7 @@ component PlainSlider inherits Rectangle {
|
||||
maximum: root.maximum;
|
||||
|
||||
changed(v) => { root.changed(v); }
|
||||
committed(v) => { root.committed(v); }
|
||||
reset => { root.reset(); }
|
||||
engaged-changed(on) => { root.drag-changed(on); }
|
||||
}
|
||||
@@ -512,10 +524,12 @@ export component GeometryPanel inherits Rectangle {
|
||||
callback flip-h-toggled();
|
||||
callback flip-v-toggled();
|
||||
callback angle-changed(float);
|
||||
/// The straightening gesture began or ended. What the auto-crop hangs
|
||||
/// off — see `DevelopSession::auto_crop_to_angle` for why it must be the
|
||||
/// end of the drag and not every frame of it.
|
||||
callback angle-drag-changed(bool);
|
||||
/// The straightening gesture is over and this is the angle to keep.
|
||||
///
|
||||
/// What the auto-crop hangs off — see `DevelopSession::auto_crop_to_angle`
|
||||
/// for why it must be the end of the drag and not every frame of it, and
|
||||
/// `PlainSlider::committed` for why it must not be `drag-changed`.
|
||||
callback angle-committed(float);
|
||||
callback angle-reset();
|
||||
callback aspect-picked(int);
|
||||
callback portrait-toggled();
|
||||
@@ -608,7 +622,7 @@ export component GeometryPanel inherits Rectangle {
|
||||
maximum: root.max-straighten;
|
||||
unit: "°";
|
||||
changed(v) => { root.angle-changed(v); }
|
||||
drag-changed(on) => { root.angle-drag-changed(on); }
|
||||
committed(v) => { root.angle-committed(v); }
|
||||
reset => { root.angle-reset(); }
|
||||
}
|
||||
|
||||
|
||||
@@ -162,12 +162,12 @@ export component AppWindow inherits Window {
|
||||
callback flip-v-toggled();
|
||||
callback straighten-changed(float);
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The straightening gesture began (`true`) or ended (`false`).
|
||||
/// The straightening gesture is over and this is the angle to keep.
|
||||
///
|
||||
/// Reported separately from the value because the auto-crop that follows a
|
||||
/// straighten has to happen once, at the end — see
|
||||
/// `DevelopSession::auto_crop_to_angle`.
|
||||
callback straighten-drag-changed(bool);
|
||||
callback straighten-committed(float);
|
||||
/// Crop, angle, rotation and flips back to neutral, leaving colour alone.
|
||||
callback framing-reset();
|
||||
|
||||
@@ -2179,8 +2179,8 @@ in property <bool> panel-visible: true;
|
||||
flip-h-toggled => { root.flip-h-toggled(); }
|
||||
flip-v-toggled => { root.flip-v-toggled(); }
|
||||
angle-changed(v) => { root.straighten-changed(v); }
|
||||
angle-drag-changed(on) => {
|
||||
root.straighten-drag-changed(on);
|
||||
angle-committed(v) => {
|
||||
root.straighten-committed(v);
|
||||
}
|
||||
angle-reset => { root.straighten-changed(0); }
|
||||
reset => { root.framing-reset(); }
|
||||
|
||||
Reference in New Issue
Block a user