olive-groves / butterfly_viewer

Side-by-side image viewer with synchronized zoom and sliding overlays. Drag and drop to instantly compare multiple images on your desktop. Very open source.
https://olive-groves.github.io/butterfly_viewer/
Other
22 stars 3 forks source link

[Bug] Changing the ruler conversion factor with existing centimeter rulers does not update those rulers #38

Closed larsmaxfield closed 3 months ago

larsmaxfield commented 4 months ago

Steps to recreate:

  1. Set the ruler conversion factor
  2. Add a centimeter ruler
  3. Change the ruler conversion factor
  4. Centimeter ruler does not update its conversion
larsmaxfield commented 4 months ago

The root cause is that when the dialog window for setting the conversion factor is accepted (and a conversion factor had previously been set), a signal is emitted containing the new conversion factor (px_per_unit) along with the units "mm". The function connected to that signal then updates the rulers which have those units. Hence, only the millimeter (mm) rulers are updated, and never the centimeter (cm) rulers. See:

# Emitter in aux_scenes.py
...
    def dialog_to_set_px_per_mm(self):
        ...
        if dialog_window.exec_() == QtWidgets.QDialog.Accepted:
            self.px_per_unit = dialog_window.px_per_unit
            if self.px_per_unit_conversion_set:
                self.changed_px_per_unit.emit("mm", self.px_per_unit)  # <-- EMITS SIGNAL

# Receiver in aux_splitview.py
    def on_changed_px_per_unit(self, unit, px_per_unit):
        ...
        for item in self._scene_main_topleft.items():
            if isinstance(item, RulerItem):
                if item.unit == unit:
                    item.set_and_refresh_px_per_unit(px_per_unit)  # <-- CATCHES SIGNAL, ONLY EXECUTES ON "mm", NOT "cm"

The fastest fix would be to change the logic of the receiver to see if the ruler is not "px" and then set it, because all non-pixel rulers use the conversion.

The better fix seems to remove the units from the signal and add a parameter to the ruler that it "relies on the conversion factor" which could then be used in the logic of the receiver to see if it should update its conversion factor. In other words, only update the rulers who "need the conversion factor".