AllYarnsAreBeautiful / ayab-desktop

The AYAB Software
http://ayab-knitting.com
GNU General Public License v3.0
56 stars 31 forks source link

[BUG] After loading a new image, the number of colours and start row do not reset to their initial values #599

Open t0mpr1c3 opened 11 months ago

t0mpr1c3 commented 11 months ago

Environment AYAB software version: current Computer/OS: all Knitting machine: - AYAB hardware: -

Describe the bug After you load a new image, the options panel is supposed to reset to the default settings. ~(It doesn't: that's a bug, and I have submitted #598 to fix it.)~ But there are other settings that don't have defaults, such as number of colours, and start row. These should also be reset (to the starting values, 2 and 1 respectively) after loading a new image because the new image might have fewer colours/rows than the previous image, so the values could be invalid.

To Reproduce Steps to reproduce the behavior:

  1. Load a three-colour image.
  2. Change number of colours to 3.
  3. Load a two-colour image.

Expected behavior The number of colours should reset to 2. The start row should reset to 1.

jonathanperret commented 2 months ago

@t0mpr1c3 I tested your PR (#654) and I can confirm it does what you describe here.

However, looking at the implementation, I discovered that something was already being done about the start row when replacing the image. In options.py we have: https://github.com/AllYarnsAreBeautiful/ayab-desktop/blob/5037932d909a7af5a5cdcba37736a0665f2cf76a/src/main/python/main/ayab/engine/options.py#L221-L230

And indeed even without your PR, the starting row is already clamped to a safe value when loading an image that is smaller than the previous one.

One behavior that is incorrect though, is that after loading a new image, regardless of whether the starting row was clamped or left as-is, the main view is not updated to reflect the starting row until it is changed or knitting starts.

Before loading a new image:

image

After loading a new image:

image

Note how the start row is still set to 10 but the view does not reflect it.

As to resetting the number of colors, I wonder if it is really necessary, since the UI never otherwise tries to prevent you from selecting more colors than exist in the image?

dl1com commented 1 month ago

Any progress on this discussion? Any actions necessary?

jonathanperret commented 1 week ago

As user feedback, I'd like to report that I was bitten last week by settings resetting when loading a new image: I wanted to knit a few images back-to-back in DBJ mode, and messed up my work (just a swatch, so nothing bad) by failing to notice the mode had switched back to single-bed when I replaced my first image by another, despite the new image having the exact same dimensions and colors as the first one.

I worked around it by changing the defaults in Preferences, but I feel it shouldn't be necessary to do so.

I wonder if it wouldn't be a better user experience to have the app try to preserve settings as much as possible when loading an image, instead of resetting them.

Even invalid settings such as too many colors for the image, or a start row higher than the image's height, could be highlighted in red rather than being forced into legal values — maybe I just loaded the wrong image, let me fix that instead of messing up my carefully selected options!

Ditto for the start/stop/alignment options. Having them stay as they are when loading a new image seems to me it would be more convenient, perhaps with a "fit" button that would reset the start/stop needles to fit the current image?