Fix Viewer slide load on backend change - take 2
Split the slide switching logic (including load/unload) into
setCurrentSlide(), while keeping setupCurrentSlide() to deal with config
only.
Change-Id: I5bd2363ffd401c1b756217f845d4dbd16d6be5d6
Reviewed-on: https://skia-review.googlesource.com/94864
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/tools/viewer/Viewer.cpp b/tools/viewer/Viewer.cpp
index 1ba32ee..f9d1c1b 100644
--- a/tools/viewer/Viewer.cpp
+++ b/tools/viewer/Viewer.cpp
@@ -173,7 +173,8 @@
const char* kRefreshStateName = "Refresh";
Viewer::Viewer(int argc, char** argv, void* platformData)
- : fRefresh(false)
+ : fCurrentSlide(-1)
+ , fRefresh(false)
, fSaveToSKP(false)
, fShowImGuiDebugWindow(false)
, fShowSlidePicker(false)
@@ -287,20 +288,10 @@
}
});
fCommands.addCommand(Window::Key::kRight, "Right", "Navigation", "Next slide", [this]() {
- int previousSlide = fCurrentSlide;
- fCurrentSlide++;
- if (fCurrentSlide >= fSlides.count()) {
- fCurrentSlide = 0;
- }
- this->setupCurrentSlide(previousSlide);
+ this->setCurrentSlide(fCurrentSlide < fSlides.count() - 1 ? fCurrentSlide + 1 : 0);
});
fCommands.addCommand(Window::Key::kLeft, "Left", "Navigation", "Previous slide", [this]() {
- int previousSlide = fCurrentSlide;
- fCurrentSlide--;
- if (fCurrentSlide < 0) {
- fCurrentSlide = fSlides.count() - 1;
- }
- this->setupCurrentSlide(previousSlide);
+ this->setCurrentSlide(fCurrentSlide > 0 ? fCurrentSlide - 1 : fSlides.count() - 1);
});
fCommands.addCommand(Window::Key::kUp, "Up", "Transform", "Zoom in", [this]() {
this->changeZoomLevel(1.f / 32.f);
@@ -389,7 +380,7 @@
// set up slides
this->initSlides();
- this->setStartupSlide();
+ this->setCurrentSlide(this->startupSlide());
if (FLAGS_list) {
this->listNames();
}
@@ -583,14 +574,13 @@
fWindow->setTitle(title.c_str());
}
-void Viewer::setStartupSlide() {
+int Viewer::startupSlide() const {
if (!FLAGS_slide.isEmpty()) {
int count = fSlides.count();
for (int i = 0; i < count; i++) {
if (fSlides[i]->getName().equals(FLAGS_slide[0])) {
- fCurrentSlide = i;
- return;
+ return i;
}
}
@@ -598,24 +588,35 @@
this->listNames();
}
- fCurrentSlide = 0;
+ return 0;
}
-void Viewer::listNames() {
- int count = fSlides.count();
+void Viewer::listNames() const {
SkDebugf("All Slides:\n");
- for (int i = 0; i < count; i++) {
- SkDebugf(" %s\n", fSlides[i]->getName().c_str());
+ for (const auto& slide : fSlides) {
+ SkDebugf(" %s\n", slide->getName().c_str());
}
}
-void Viewer::setupCurrentSlide(int previousSlide) {
- if (fCurrentSlide == previousSlide) {
- return; // no change; do nothing
- }
- // prepare dimensions for image slides
- fSlides[fCurrentSlide]->load(SkIntToScalar(fWindow->width()), SkIntToScalar(fWindow->height()));
+void Viewer::setCurrentSlide(int slide) {
+ SkASSERT(slide >= 0 && slide < fSlides.count());
+ if (slide == fCurrentSlide) {
+ return;
+ }
+
+ if (fCurrentSlide >= 0) {
+ fSlides[fCurrentSlide]->unload();
+ }
+
+ fSlides[slide]->load(SkIntToScalar(fWindow->width()),
+ SkIntToScalar(fWindow->height()));
+ fCurrentSlide = slide;
+ this->setupCurrentSlide();
+}
+
+void Viewer::setupCurrentSlide() {
+ // prepare dimensions for image slides
fGesture.resetTouchState();
fDefaultMatrix.reset();
@@ -635,9 +636,6 @@
this->updateTitle();
this->updateUIState();
- if (previousSlide >= 0) {
- fSlides[previousSlide]->unload();
- }
fStatsLayer.resetMeasurements();
@@ -823,7 +821,7 @@
void Viewer::onBackendCreated() {
this->updateTitle();
this->updateUIState();
- this->setupCurrentSlide(-1);
+ this->setupCurrentSlide();
fStatsLayer.resetMeasurements();
fWindow->show();
fWindow->inval();
@@ -1076,11 +1074,9 @@
}
}
- int previousSlide = fCurrentSlide;
if (ImGui::ListBox("", &filteredIndex, filteredSlideNames.begin(),
filteredSlideNames.size(), 20)) {
- fCurrentSlide = filteredSlideIndices[filteredIndex];
- setupCurrentSlide(previousSlide);
+ this->setCurrentSlide(filteredSlideIndices[filteredIndex]);
}
}
@@ -1310,19 +1306,14 @@
// For example, after slide change, updateUIState is called inside setupCurrentSlide;
// after backend change, updateUIState is called in this function.
if (stateName.equals(kSlideStateName)) {
- int previousSlide = fCurrentSlide;
- fCurrentSlide = 0;
- for(auto slide : fSlides) {
- if (slide->getName().equals(stateValue)) {
- this->setupCurrentSlide(previousSlide);
- break;
+ for (int i = 0; i < fSlides.count(); ++i) {
+ if (fSlides[i]->getName().equals(stateValue)) {
+ this->setCurrentSlide(i);
+ return;
}
- fCurrentSlide++;
}
- if (fCurrentSlide >= fSlides.count()) {
- fCurrentSlide = previousSlide;
- SkDebugf("Slide not found: %s", stateValue.c_str());
- }
+
+ SkDebugf("Slide not found: %s", stateValue.c_str());
} else if (stateName.equals(kBackendStateName)) {
for (int i = 0; i < sk_app::Window::kBackendTypeCount; i++) {
if (stateValue.equals(kBackendTypeStrings[i])) {
diff --git a/tools/viewer/Viewer.h b/tools/viewer/Viewer.h
index 7d4980c..5067952 100644
--- a/tools/viewer/Viewer.h
+++ b/tools/viewer/Viewer.h
@@ -49,9 +49,10 @@
void updateTitle();
void setBackend(sk_app::Window::BackendType);
void setColorMode(ColorMode);
- void setStartupSlide();
- void setupCurrentSlide(int previousSlide);
- void listNames();
+ int startupSlide() const;
+ void setCurrentSlide(int);
+ void setupCurrentSlide();
+ void listNames() const;
void updateUIState();