From d5b1934464993164a1fbed59ea142115833ed3ab Mon Sep 17 00:00:00 2001 From: gulis1 Date: Fri, 25 Nov 2022 17:28:53 +0100 Subject: [PATCH 1/5] Changed the undo_history implementation. Every 20 (configurable) operations the current state is saved. --- src/history_manager.py | 157 +++++++++++++++++++++++++---------------- src/image.py | 4 +- 2 files changed, 99 insertions(+), 62 deletions(-) diff --git a/src/history_manager.py b/src/history_manager.py index 410d4c36..8a0d72f9 100644 --- a/src/history_manager.py +++ b/src/history_manager.py @@ -5,6 +5,35 @@ ################################################################################ +# +class State(): + + def __init__(self, saved_state, max_operations=20): + + # A saved state operation, containing a pixbuf, width, height... + self.initial_state = saved_state + + # The next operations following self.initial state. + # This list can have at most max_operations items. + self.operations = [] + + self._max_operations = max_operations + + # Convenience methods. + + def pop_last_operation(self): + return self.operations.pop() if len(self.operations) > 0 else None + + def add_operation(self, op): + self.operations.append(op) + + def is_empty(self): + return len(self.operations) == 0 + + def is_full(self): + return len(self.operations) == self._max_operations + + class DrHistoryManager(): __gtype_name__ = 'DrHistoryManager' @@ -21,14 +50,15 @@ def get_saved(self): # in all situations around a saving return self._is_saved + def get_initial_operation(self): + return self._undo_history[-1].initial_state + + def empty_history(self): """Probably useless way to explicitely 'forget' the objects. It doesn't really free the memory, but it kinda helps i suppose.""" - for op in self._undo_history: - self._delete_operation(op) - for op in self._redo_history: - self._delete_operation(op) - self._delete_operation(self.initial_operation) + self._undo_history.clear() + self._redo_history.clear() def _delete_operation(self, op): for key in op: @@ -40,26 +70,31 @@ def _delete_operation(self, op): # Controls accessed by DrImage ############################################# def try_undo(self): + if self._operation_is_ongoing(): self._image.active_tool().cancel_ongoing_operation() return - if len(self._undo_history) > 0: - last_op = self._undo_history.pop() - self._redo_history.append(last_op) - self._rebuild_from_history_async() - self._image.update_history_sensitivity() + + undone_op = self._undo_history[-1].pop_last_operation() + if undone_op is not None: + self._redo_history.append(undone_op) + + # Never pop the first saved state. + if self._undo_history[-1].is_empty() and len(self._undo_history) > 1: + self._undo_history.pop() + + self._rebuild_from_history_async() + self._image.update_history_sensitivity() def try_redo(self, *args): operation = self._redo_history.pop() - if operation['tool_id'] is None: - self._undo_history.append(operation) - self._image.restore_last_state() - else: - self._get_tool(operation['tool_id']).apply_operation(operation) + self._get_tool(operation['tool_id']).apply_operation(operation) + self.add_operation(operation) def can_undo(self): # XXX incorrect si ya des states et qu'on redo - return (len(self._undo_history) > 0) or self._operation_is_ongoing() + return (len(self._undo_history[-1].operations) > 0 + or self._operation_is_ongoing()) def can_redo(self): # XXX incorrect si ya des states ? @@ -95,14 +130,18 @@ def rewind_history(self): # Serialized operations #################################################### def add_operation(self, operation): + self._image.set_surface_as_stable_pixbuf() - # print('add_operation_to_history') - # print(operation['tool_id']) - # if 'select' in operation['tool_id']: - # print(operation['operation_type']) - # print('-----------------------------------') + + + if self._undo_history[-1].is_full(): + self.add_state(self._image.main_pixbuf.copy()) + + self._undo_history[-1].add_operation(operation) + # TODO: clear redo_history after a new operation is + # drawn (manually) by the user. + # self._redo_history.clear() self._is_saved = False - self._undo_history.append(operation) ############################################################################ # Cached pixbufs ########################################################### @@ -112,54 +151,55 @@ def set_initial_operation(self, rgba_array, pixbuf, width, height): g = float(rgba_array[1]) b = float(rgba_array[2]) a = float(rgba_array[3]) - self.initial_operation = { + initial_operation = { 'tool_id': None, 'pixbuf': pixbuf, 'rgba': Gdk.RGBA(red=r, green=g, blue=b, alpha=a), 'width': width, 'height': height } + self._undo_history.append(State(initial_operation)) def add_state(self, pixbuf): + if pixbuf is None: # Context: an error message - raise Exception(_("Attempt to save an invalid state")) - self._undo_history.append({ + raise Exception("Attempt to save an invalid state") + + new_state = { 'tool_id': None, 'pixbuf': pixbuf, 'width': pixbuf.get_width(), 'height': pixbuf.get_height() - }) + } + + self._undo_history.append(State(new_state)) self._is_saved = True def has_initial_pixbuf(self): - return self.initial_operation['pixbuf'] is not None + return len(self._undo_history) > 0 - def get_last_saved_state(self): - index = self._get_last_state_index(False) - if index == -1: - return self.initial_operation - else: - return self._undo_history[index] + def get_last_saved_state(self): + return self._undo_history[-1].initial_state - def _get_last_state_index(self, allow_yeeting_states): - """Return the index of the last "state" operation (dict whose 'tool_id' - value is None) in the undo-history. If there is no such operation, the - returned index is -1 which means the only known state is the - self.initial_operation attribute.""" + # def _get_last_state_index(self, allow_yeeting_states): + # """Return the index of the last "state" operation (dict whose 'tool_id' + # value is None) in the undo-history. If there is no such operation, the + # returned index is -1 which means the only known state is the + # self.initial_operation attribute.""" - returned_index = -1 - nbPixbufs = 0 - for op in self._undo_history: - if op['tool_id'] is None: - last_saved_pixbuf_op = op - returned_index = self._undo_history.index(op) - nbPixbufs += 1 + # returned_index = -1 + # nbPixbufs = 0 + # for op in self._undo_history: + # if op['tool_id'] is None: + # last_saved_pixbuf_op = op + # returned_index = self._undo_history.index(op) + # nbPixbufs += 1 - # TODO if there are too many pixbufs in the history, remove a few ones - # Issue #200, needs more design because saving can change the data + # # TODO if there are too many pixbufs in the history, remove a few ones + # # Issue #200, needs more design because saving can change the data - # print("returned_index : " + str(returned_index)) - return returned_index + # # print("returned_index : " + str(returned_index)) + # return returned_index ############################################################################ # Other private methods #################################################### @@ -182,17 +222,14 @@ def _rebuild_from_history(self, async_cb_data={}): return False self._waiting_for_rebuild = False - last_save_index = self._get_last_state_index(True) self._image.restore_last_state() - history = self._undo_history.copy() - self._undo_history = [] - for op in history: - if history.index(op) > last_save_index: - # print("do", op['tool_id']) - self._get_tool(op['tool_id']).simple_apply_operation(op) - else: - # print("skip", op['tool_id']) - self._undo_history.append(op) + + x = self._undo_history[-1].operations + self._undo_history[-1].operations = [] + + for op in x: + self._get_tool(op['tool_id']).simple_apply_operation(op) + self._image.update() return False @@ -204,7 +241,7 @@ def _get_tool(self, tool_id): if tool_id in all_tools: return all_tools[tool_id] else: - self._image.window.reveal_action_report(_("Error: no tool '%s'" % tool_id)) + self._image.window.reveal_action_report("Error: no tool '%s'" % tool_id) # no raise: this may happen normally if last_save_index is incorrect ############################################################################ diff --git a/src/image.py b/src/image.py index 60d104f1..3da4d029 100644 --- a/src/image.py +++ b/src/image.py @@ -180,7 +180,7 @@ def restore_last_state(self): self._apply_state(last_saved_pixbuf_op) def reset_to_initial_pixbuf(self): - self._apply_state(self._history.initial_operation) + self._apply_state(self._history.get_initial_operation()) self._history.rewind_history() def _apply_state(self, state_op): @@ -379,7 +379,7 @@ def should_replace(self): return not self._history.has_initial_pixbuf() def get_initial_rgba(self): - return self._history.initial_operation['rgba'] + return self._history.get_initial_operation()['rgba'] ############################################################################ # Misc ? ################################################################### From 12ca5a051374cadebac0e0a35ef51dcf497a58a1 Mon Sep 17 00:00:00 2001 From: gulis1 Date: Sat, 26 Nov 2022 19:33:58 +0100 Subject: [PATCH 2/5] Cleanup of last commit Added some comments and removed some comented code. --- src/history_manager.py | 60 +++++++++++++----------------------------- src/image.py | 4 +-- 2 files changed, 20 insertions(+), 44 deletions(-) diff --git a/src/history_manager.py b/src/history_manager.py index 8a0d72f9..905aaafe 100644 --- a/src/history_manager.py +++ b/src/history_manager.py @@ -6,12 +6,13 @@ ################################################################################ # + class State(): - def __init__(self, saved_state, max_operations=20): + def __init__(self, stored_state, max_operations=20): # A saved state operation, containing a pixbuf, width, height... - self.initial_state = saved_state + self.initial_state = stored_state # The next operations following self.initial state. # This list can have at most max_operations items. @@ -70,34 +71,32 @@ def _delete_operation(self, op): # Controls accessed by DrImage ############################################# def try_undo(self): - + if self._operation_is_ongoing(): self._image.active_tool().cancel_ongoing_operation() return - + undone_op = self._undo_history[-1].pop_last_operation() if undone_op is not None: + # Dont save reloads from disk to redo history self._redo_history.append(undone_op) - # Never pop the first saved state. - if self._undo_history[-1].is_empty() and len(self._undo_history) > 1: - self._undo_history.pop() - - self._rebuild_from_history_async() - self._image.update_history_sensitivity() + if self._undo_history[-1].is_empty() and len(self._undo_history) > 1: + self._undo_history.pop() + + self._rebuild_from_history_async() + self._image.update_history_sensitivity() def try_redo(self, *args): operation = self._redo_history.pop() self._get_tool(operation['tool_id']).apply_operation(operation) - self.add_operation(operation) def can_undo(self): - # XXX incorrect si ya des states et qu'on redo - return (len(self._undo_history[-1].operations) > 0 - or self._operation_is_ongoing()) + return (len(self._undo_history) > 1 or + len(self._undo_history[0].operations) > 0 or + self._operation_is_ongoing()) def can_redo(self): - # XXX incorrect si ya des states ? return len(self._redo_history) > 0 # def update_history_actions_labels(self): @@ -132,21 +131,19 @@ def rewind_history(self): def add_operation(self, operation): self._image.set_surface_as_stable_pixbuf() - - if self._undo_history[-1].is_full(): self.add_state(self._image.main_pixbuf.copy()) self._undo_history[-1].add_operation(operation) - # TODO: clear redo_history after a new operation is - # drawn (manually) by the user. - # self._redo_history.clear() self._is_saved = False + self._image.update_title() + ############################################################################ # Cached pixbufs ########################################################### def set_initial_operation(self, rgba_array, pixbuf, width, height): + r = float(rgba_array[0]) g = float(rgba_array[1]) b = float(rgba_array[2]) @@ -160,7 +157,6 @@ def set_initial_operation(self, rgba_array, pixbuf, width, height): self._undo_history.append(State(initial_operation)) def add_state(self, pixbuf): - if pixbuf is None: # Context: an error message raise Exception("Attempt to save an invalid state") @@ -178,29 +174,9 @@ def add_state(self, pixbuf): def has_initial_pixbuf(self): return len(self._undo_history) > 0 - def get_last_saved_state(self): + def get_last_stored_state(self): return self._undo_history[-1].initial_state - # def _get_last_state_index(self, allow_yeeting_states): - # """Return the index of the last "state" operation (dict whose 'tool_id' - # value is None) in the undo-history. If there is no such operation, the - # returned index is -1 which means the only known state is the - # self.initial_operation attribute.""" - - # returned_index = -1 - # nbPixbufs = 0 - # for op in self._undo_history: - # if op['tool_id'] is None: - # last_saved_pixbuf_op = op - # returned_index = self._undo_history.index(op) - # nbPixbufs += 1 - - # # TODO if there are too many pixbufs in the history, remove a few ones - # # Issue #200, needs more design because saving can change the data - - # # print("returned_index : " + str(returned_index)) - # return returned_index - ############################################################################ # Other private methods #################################################### diff --git a/src/image.py b/src/image.py index 3da4d029..d469ee77 100644 --- a/src/image.py +++ b/src/image.py @@ -176,7 +176,7 @@ def _new_blank_pixbuf(self, w, h): def restore_last_state(self): """Set the last saved pixbuf from the history as the main_pixbuf. This is used to rebuild the picture from its history.""" - last_saved_pixbuf_op = self._history.get_last_saved_state() + last_saved_pixbuf_op = self._history.get_last_stored_state() self._apply_state(last_saved_pixbuf_op) def reset_to_initial_pixbuf(self): @@ -229,7 +229,6 @@ def reload_from_disk(self): self.window.update_picture_title() self.use_stable_pixbuf() self.update() - self.remember_current_state() def try_load_file(self, gfile): try: @@ -312,6 +311,7 @@ def update_title(self): if not self.is_saved(): main_title = "*" + main_title self.set_tab_label(main_title) + return main_title def get_filename_for_display(self): From d3ad9709b7dbbaeb1e861b760987e92fd5a24a8c Mon Sep 17 00:00:00 2001 From: gulis1 Date: Sat, 31 Dec 2022 20:25:26 +0100 Subject: [PATCH 3/5] Moved state class to a separate file. --- src/history_manager.py | 35 ++++------------------------------- src/history_state.py | 28 ++++++++++++++++++++++++++++ src/meson.build | 1 + 3 files changed, 33 insertions(+), 31 deletions(-) create mode 100644 src/history_state.py diff --git a/src/history_manager.py b/src/history_manager.py index 905aaafe..b68cd524 100644 --- a/src/history_manager.py +++ b/src/history_manager.py @@ -1,40 +1,13 @@ # Licensed under GPL3 https://github.com/maoschanz/drawing/blob/master/LICENSE -from gi.repository import Gdk, Gio, GdkPixbuf, GLib +from gi.repository import Gdk, GLib +from .history_state import DrHistoryState # from .abstract_tool import WrongToolIdException ################################################################################ # -class State(): - - def __init__(self, stored_state, max_operations=20): - - # A saved state operation, containing a pixbuf, width, height... - self.initial_state = stored_state - - # The next operations following self.initial state. - # This list can have at most max_operations items. - self.operations = [] - - self._max_operations = max_operations - - # Convenience methods. - - def pop_last_operation(self): - return self.operations.pop() if len(self.operations) > 0 else None - - def add_operation(self, op): - self.operations.append(op) - - def is_empty(self): - return len(self.operations) == 0 - - def is_full(self): - return len(self.operations) == self._max_operations - - class DrHistoryManager(): __gtype_name__ = 'DrHistoryManager' @@ -154,7 +127,7 @@ def set_initial_operation(self, rgba_array, pixbuf, width, height): 'rgba': Gdk.RGBA(red=r, green=g, blue=b, alpha=a), 'width': width, 'height': height } - self._undo_history.append(State(initial_operation)) + self._undo_history.append(DrHistoryState(initial_operation)) def add_state(self, pixbuf): if pixbuf is None: @@ -168,7 +141,7 @@ def add_state(self, pixbuf): 'height': pixbuf.get_height() } - self._undo_history.append(State(new_state)) + self._undo_history.append(DrHistoryState(new_state)) self._is_saved = True def has_initial_pixbuf(self): diff --git a/src/history_state.py b/src/history_state.py new file mode 100644 index 00000000..f4433063 --- /dev/null +++ b/src/history_state.py @@ -0,0 +1,28 @@ +# Licensed under GPL3 https://github.com/maoschanz/drawing/blob/master/LICENSE + +class DrHistoryState(): + + def __init__(self, stored_state, max_operations=20): + + # A saved state operation, containing a pixbuf, width, height... + self.initial_state = stored_state + + # The next operations following self.initial state. + # This list can have at most max_operations items. + self.operations = [] + + self._max_operations = max_operations + + # Convenience methods. + + def pop_last_operation(self): + return self.operations.pop() if len(self.operations) > 0 else None + + def add_operation(self, op): + self.operations.append(op) + + def is_empty(self): + return len(self.operations) == 0 + + def is_full(self): + return len(self.operations) == self._max_operations \ No newline at end of file diff --git a/src/meson.build b/src/meson.build index 951f00b4..43cab792 100644 --- a/src/meson.build +++ b/src/meson.build @@ -42,6 +42,7 @@ drawing_sources = [ 'tools_initializer.py', 'image.py', + 'history_state.py', 'history_manager.py', 'printing_manager.py', 'saving_manager.py', From 9c82f97a98acc469f2d8d2bf5ebda114b1532138 Mon Sep 17 00:00:00 2001 From: gulis1 Date: Sat, 31 Dec 2022 21:29:03 +0100 Subject: [PATCH 4/5] Restored removed comments and gettext. --- src/history_manager.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/history_manager.py b/src/history_manager.py index b68cd524..c8153f2b 100644 --- a/src/history_manager.py +++ b/src/history_manager.py @@ -27,19 +27,10 @@ def get_saved(self): def get_initial_operation(self): return self._undo_history[-1].initial_state - def empty_history(self): - """Probably useless way to explicitely 'forget' the objects. It doesn't - really free the memory, but it kinda helps i suppose.""" self._undo_history.clear() self._redo_history.clear() - def _delete_operation(self, op): - for key in op: - op[key] = None - op = {} - op = None - ############################################################################ # Controls accessed by DrImage ############################################# @@ -51,7 +42,11 @@ def try_undo(self): undone_op = self._undo_history[-1].pop_last_operation() if undone_op is not None: - # Dont save reloads from disk to redo history + # Dont save reloads from disk to the redo_history. + # TODO: The reason is the redo can cause some weird + # behaviour with the undo button if it is done just + # after reloading. It might be able to get fixed by + # rethinking the try_redo method a bit. self._redo_history.append(undone_op) if self._undo_history[-1].is_empty() and len(self._undo_history) > 1: @@ -105,6 +100,8 @@ def add_operation(self, operation): self._image.set_surface_as_stable_pixbuf() if self._undo_history[-1].is_full(): + # TODO if there are too many pixbufs in the history, remove a few ones + # Issue #200, needs more design because saving can change the data self.add_state(self._image.main_pixbuf.copy()) self._undo_history[-1].add_operation(operation) @@ -132,7 +129,7 @@ def set_initial_operation(self, rgba_array, pixbuf, width, height): def add_state(self, pixbuf): if pixbuf is None: # Context: an error message - raise Exception("Attempt to save an invalid state") + raise Exception(_("Attempt to save an invalid state")) new_state = { 'tool_id': None, @@ -190,7 +187,7 @@ def _get_tool(self, tool_id): if tool_id in all_tools: return all_tools[tool_id] else: - self._image.window.reveal_action_report("Error: no tool '%s'" % tool_id) + self._image.window.reveal_action_report(_("Error: no tool '%s'" % tool_id)) # no raise: this may happen normally if last_save_index is incorrect ############################################################################ From b9762f2b10d7f362d275385b4d473e50fde029cb Mon Sep 17 00:00:00 2001 From: gulis1 Date: Sat, 31 Dec 2022 23:35:33 +0100 Subject: [PATCH 5/5] Added entry in the "about" section. --- src/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.py b/src/main.py index 1891fdce..ec82e88c 100644 --- a/src/main.py +++ b/src/main.py @@ -292,7 +292,7 @@ def on_about(self, *args): """Action callback, showing the "about" dialog.""" about_dialog = Gtk.AboutDialog(transient_for=self.props.active_window, copyright="© 2018-2022 Romain F. T.", - authors=["Romain F. T.", "Fábio Colacio", "Alexis Lozano"], + authors=["Romain F. T.", "Fábio Colacio", "Alexis Lozano", "Julián Cámara"], # To tranlators: "translate" this by a list of your names (one name # per line), they will be displayed in the "about" dialog translator_credits=_("translator-credits"),