From 64b92a95ac738ee17882ca916c940a6126a9f0a7 Mon Sep 17 00:00:00 2001 From: arch1t3cht Date: Thu, 11 May 2023 23:51:23 +0200 Subject: [PATCH 1/3] folding: Simplify fold operation code --- src/ass_file.h | 7 ---- src/fold_controller.cpp | 76 ++++++++++++++++++++++------------------- src/fold_controller.h | 19 ++--------- src/grid_column.cpp | 4 +-- 4 files changed, 44 insertions(+), 62 deletions(-) diff --git a/src/ass_file.h b/src/ass_file.h index 719b28bb8..1ccbf627b 100644 --- a/src/ass_file.h +++ b/src/ass_file.h @@ -54,13 +54,6 @@ struct ExtradataEntry { std::string value; }; -// Both start and end are inclusive -struct LineFold { - int start; - int end; - bool collapsed; -}; - struct AssFileCommit { wxString const& message; int *commit_id; diff --git a/src/fold_controller.cpp b/src/fold_controller.cpp index a0b578759..7c4abd50c 100644 --- a/src/fold_controller.cpp +++ b/src/fold_controller.cpp @@ -79,16 +79,13 @@ void FoldController::AddFold(AssDialogue& start, AssDialogue& end, bool collapse } } -bool FoldController::DoForAllFolds(bool action(AssDialogue& line)) { +void FoldController::DoForAllFolds(std::function action) { for (AssDialogue& line : context->ass->Events) { if (line.Fold.valid) { - bool result = action(line); + action(line); UpdateLineExtradata(line); - if (result) - return true; } } - return false; } void FoldController::FixFoldsPreCommit(int type, const AssDialogue *single_line) { @@ -98,24 +95,22 @@ void FoldController::FixFoldsPreCommit(int type, const AssDialogue *single_line) } // For each line in lines, applies action() to the opening delimiter of the innermost fold containing this line. -// Returns true as soon as any action() call returned true. // // In general, this can leave the folds in an inconsistent state, so unless action() is read-only this should always // be followed by a commit. -bool FoldController::DoForFoldsAt(std::vector const& lines, bool action(AssDialogue& line)) { +void FoldController::DoForFoldsAt(std::vector const& lines, std::function action) { + std::map visited; for (AssDialogue *line : lines) { if (line->Fold.parent != nullptr && !(line->Fold.valid && !line->Fold.side)) { line = line->Fold.parent; } - if (!line->Fold.visited) { - bool result = action(*line); - UpdateLineExtradata(*line); - if (result) - return true; - } - line->Fold.visited = true; + if (visited.count(line->Row)) + continue; + + action(*line); + UpdateLineExtradata(*line); + visited[line->Row] = true; } - return false; } void FoldController::UpdateFoldInfo() { @@ -263,7 +258,6 @@ void FoldController::LinkFolds() { line->Fold.parent = foldStack.empty() ? nullptr : foldStack.back(); line->Fold.nextVisible = nullptr; line->Fold.visible = highestFolded > (int) foldStack.size(); - line->Fold.visited = false; line->Fold.visibleRow = visibleRow; if (line->Fold.visible) { @@ -299,56 +293,68 @@ int FoldController::GetMaxDepth() { return maxdepth; } -bool FoldController::ActionHasFold(AssDialogue& line) { return line.Fold.valid; } - -bool FoldController::ActionClearFold(AssDialogue& line) { line.Fold.extraExists = false; line.Fold.valid = false; return false; } - -bool FoldController::ActionOpenFold(AssDialogue& line) { line.Fold.collapsed = false; return false; } - -bool FoldController::ActionCloseFold(AssDialogue& line) { line.Fold.collapsed = true; return false; } - -bool FoldController::ActionToggleFold(AssDialogue& line) { line.Fold.collapsed = !line.Fold.collapsed; return false; } - void FoldController::ClearAllFolds() { - FoldController::DoForAllFolds(FoldController::ActionClearFold); + DoForAllFolds([&](AssDialogue &line) { + line.Fold.extraExists = false; line.Fold.valid = false; + }); context->ass->Commit(_("clear all folds"), AssFile::COMMIT_FOLD); } void FoldController::OpenAllFolds() { - FoldController::DoForAllFolds(FoldController::ActionOpenFold); + DoForAllFolds([&](AssDialogue &line) { + line.Fold.collapsed = false; + }); context->ass->Commit(_("open all folds"), AssFile::COMMIT_FOLD); } void FoldController::CloseAllFolds() { - FoldController::DoForAllFolds(FoldController::ActionCloseFold); + DoForAllFolds([&](AssDialogue &line) { + line.Fold.collapsed = true; + }); context->ass->Commit(_("close all folds"), AssFile::COMMIT_FOLD); } bool FoldController::HasFolds() { - return FoldController::DoForAllFolds(FoldController::ActionHasFold); + bool hasfold = false; + DoForAllFolds([&](AssDialogue &line) { + hasfold = hasfold || line.Fold.valid; + }); + return hasfold; } void FoldController::ClearFoldsAt(std::vector const& lines) { - FoldController::DoForFoldsAt(lines, FoldController::ActionClearFold); + DoForFoldsAt(lines, [&](AssDialogue &line) { + line.Fold.extraExists = false; line.Fold.valid = false; + }); context->ass->Commit(_("clear folds"), AssFile::COMMIT_FOLD); } void FoldController::OpenFoldsAt(std::vector const& lines) { - FoldController::DoForFoldsAt(lines, FoldController::ActionOpenFold); + DoForFoldsAt(lines, [&](AssDialogue &line) { + line.Fold.collapsed = false; + }); context->ass->Commit(_("open folds"), AssFile::COMMIT_FOLD); } void FoldController::CloseFoldsAt(std::vector const& lines) { - FoldController::DoForFoldsAt(lines, FoldController::ActionCloseFold); + DoForFoldsAt(lines, [&](AssDialogue &line) { + line.Fold.collapsed = true; + }); context->ass->Commit(_("close folds"), AssFile::COMMIT_FOLD); } void FoldController::ToggleFoldsAt(std::vector const& lines) { - FoldController::DoForFoldsAt(lines, FoldController::ActionToggleFold); + DoForFoldsAt(lines, [&](AssDialogue &line) { + line.Fold.collapsed = !line.Fold.collapsed; + }); context->ass->Commit(_("toggle folds"), AssFile::COMMIT_FOLD); } bool FoldController::AreFoldsAt(std::vector const& lines) { - return FoldController::DoForFoldsAt(lines, FoldController::ActionHasFold); + bool hasfold = false; + DoForFoldsAt(lines, [&](AssDialogue &line) { + hasfold = hasfold || line.Fold.valid; + }); + return hasfold; } diff --git a/src/fold_controller.h b/src/fold_controller.h index c53db5d82..f1cea0889 100644 --- a/src/fold_controller.h +++ b/src/fold_controller.h @@ -56,9 +56,6 @@ class FoldInfo { /// False if a fold is started here, true otherwise. bool side = false; - - // Used in DoForFoldsAt to ensure each line is visited only once - bool visited = false; /// Whether the line is currently visible bool visible = true; @@ -106,24 +103,12 @@ class FoldController { void RawAddFold(AssDialogue& start, AssDialogue& end, bool collapsed); - bool DoForFoldsAt(std::vector const& lines, bool action(AssDialogue& line)); + void DoForFoldsAt(std::vector const& lines, std::function action); - bool DoForAllFolds(bool action(AssDialogue& line)); + void DoForAllFolds(std::function action); void FixFoldsPreCommit(int type, const AssDialogue *single_line); - // These are used for the DoForAllFolds action and should not be used as ordinary getters/setters - - static bool ActionHasFold(AssDialogue& line); - - static bool ActionClearFold(AssDialogue& line); - - static bool ActionOpenFold(AssDialogue& line); - - static bool ActionCloseFold(AssDialogue& line); - - static bool ActionToggleFold(AssDialogue& line); - /// Updates the line's extradata entry from the values in FoldInfo. Used after actions like toggling folds. void UpdateLineExtradata(AssDialogue& line); diff --git a/src/grid_column.cpp b/src/grid_column.cpp index 08e165a41..a7db7d6e8 100644 --- a/src/grid_column.cpp +++ b/src/grid_column.cpp @@ -150,9 +150,7 @@ struct GridColumnFolds final : GridColumn { bool OnMouseEvent(AssDialogue *d, agi::Context *c, wxMouseEvent &event) const override { if ((event.LeftDown() || event.LeftDClick()) && !event.ShiftDown() && !event.CmdDown() && !event.AltDown()) { if (d->Fold.hasFold() && !d->Fold.isEnd()) { - std::vector lines; - lines.push_back(d); - c->foldController->ToggleFoldsAt(lines); + c->foldController->ToggleFoldsAt({d}); return true; } } From 58d6ab520bbeed3f57db880d905d9702e5c1db65 Mon Sep 17 00:00:00 2001 From: arch1t3cht Date: Thu, 1 Jun 2023 22:53:30 +0200 Subject: [PATCH 2/3] folding: Also update counterpart for fold operations This wasn't necessary before since the internal representation of folds is be checked for consistency after each commit, but after the switch to extradata fold operations would leave the extradata in an invalid state. This isn't technically a problem, but it does leave more extradata entries lying around than necessary, and it can trip up automation scripts that aren't prepared for inconsistent fold state. --- src/fold_controller.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/fold_controller.cpp b/src/fold_controller.cpp index 7c4abd50c..ff2df88dd 100644 --- a/src/fold_controller.cpp +++ b/src/fold_controller.cpp @@ -326,6 +326,10 @@ bool FoldController::HasFolds() { void FoldController::ClearFoldsAt(std::vector const& lines) { DoForFoldsAt(lines, [&](AssDialogue &line) { line.Fold.extraExists = false; line.Fold.valid = false; + if (line.Fold.counterpart) { + line.Fold.counterpart->Fold.extraExists = false; + line.Fold.counterpart->Fold.valid = false; + } }); context->ass->Commit(_("clear folds"), AssFile::COMMIT_FOLD); } @@ -333,6 +337,8 @@ void FoldController::ClearFoldsAt(std::vector const& lines) { void FoldController::OpenFoldsAt(std::vector const& lines) { DoForFoldsAt(lines, [&](AssDialogue &line) { line.Fold.collapsed = false; + if (line.Fold.counterpart) + line.Fold.counterpart->Fold.collapsed = line.Fold.collapsed; }); context->ass->Commit(_("open folds"), AssFile::COMMIT_FOLD); } @@ -340,6 +346,8 @@ void FoldController::OpenFoldsAt(std::vector const& lines) { void FoldController::CloseFoldsAt(std::vector const& lines) { DoForFoldsAt(lines, [&](AssDialogue &line) { line.Fold.collapsed = true; + if (line.Fold.counterpart) + line.Fold.counterpart->Fold.collapsed = line.Fold.collapsed; }); context->ass->Commit(_("close folds"), AssFile::COMMIT_FOLD); } @@ -347,6 +355,8 @@ void FoldController::CloseFoldsAt(std::vector const& lines) { void FoldController::ToggleFoldsAt(std::vector const& lines) { DoForFoldsAt(lines, [&](AssDialogue &line) { line.Fold.collapsed = !line.Fold.collapsed; + if (line.Fold.counterpart) + line.Fold.counterpart->Fold.collapsed = line.Fold.collapsed; }); context->ass->Commit(_("toggle folds"), AssFile::COMMIT_FOLD); } From b41f6bde71bf32a2faf11830d302d3eb53cb68e6 Mon Sep 17 00:00:00 2001 From: arch1t3cht Date: Thu, 1 Jun 2023 23:35:20 +0200 Subject: [PATCH 3/3] Don't immediately delete unused extradata entries Instead, count how many consecutive times the entry has been found to be unused and delete it once that count exceeds a limit. This will prevent excessive reallocating of extradata ID's in applications like folding. --- src/ass_file.cpp | 10 ++++++++-- src/ass_file.h | 1 + src/ass_parser.cpp | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/ass_file.cpp b/src/ass_file.cpp index 6d36d37a5..e39e9fc1a 100644 --- a/src/ass_file.cpp +++ b/src/ass_file.cpp @@ -236,7 +236,7 @@ uint32_t AssFile::AddExtradata(std::string const& key, std::string const& value) return data.id; } } - Extradata.push_back(ExtradataEntry{next_extradata_id, key, value}); + Extradata.push_back(ExtradataEntry{next_extradata_id, 0, key, value}); return next_extradata_id++; // return old value, then post-increment } @@ -334,10 +334,16 @@ void AssFile::CleanExtradata() { } } + for (ExtradataEntry &e : Extradata) { + if (ids_used.count(e.id)) + e.expiration_counter = 0; + else + e.expiration_counter++; + } if (ids_used.size() != Extradata.size()) { // Erase all no-longer-used extradata entries Extradata.erase(std::remove_if(begin(Extradata), end(Extradata), [&](ExtradataEntry const& e) { - return !ids_used.count(e.id); + return e.expiration_counter >= 10; }), end(Extradata)); } } diff --git a/src/ass_file.h b/src/ass_file.h index 1ccbf627b..de0165148 100644 --- a/src/ass_file.h +++ b/src/ass_file.h @@ -50,6 +50,7 @@ using EntryList = typename boost::intrusive::make_listnext_extradata_id = std::max(id+1, target->next_extradata_id); - target->Extradata.push_back(ExtradataEntry{id, std::move(key), std::move(value)}); + target->Extradata.push_back(ExtradataEntry{id, 0, std::move(key), std::move(value)}); } }