Skip to content

Conversation

@gwhitney
Copy link
Collaborator

@gwhitney gwhitney commented Oct 30, 2024

Prior to this change, the browser 'back' button would always update the
URL, but the visualization would typically not change if you were going
from one scope URL (/?name=Blah&...) to another.

This PR remedies the situation by watching for route changes in the
Scope view, and reloading the new visualizer and sequence into the
specimen already being viewed. Implementing this tactic requires the
ability to completely change the sequence and visualizer of a specimen,
which previously did not exist.

The change also enables delaying specimen initialization from Vue setup()
time to just before the component is mounted. As a result, the Vue setup
is not asynchronous, so the experimental Vue <Suspense> tag is no longer
needed.

Adds a test that the back button actually took effect, to the existing
test that changes a parameter. (The extended test definitely failed in
ui2 prior to this PR.)

Uses the router to redirect from an empty URL to the last-viewed specimen,
rather than the Scope component; this change avoids reloading that specimen
when the URL is updated to reflect what specimen is being viewed.

Finally, fixes a small previously-existing bug in which the Differences
visualizer could never display more terms than were available in its last
parameter settings, even if you changed the parameters to make more
terms available.

Resolves #414.

Given the large number of internal changes to Specimen.ts, please
in review do play with a veriety of visualizations to make sure all seems
to be working well.


By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.

  Prior to this change, the browser 'back' button would always update the
  URL, but the visualization would typically not change if you were going
  from one scope URL (`/?name=Blah&...`) to another.

  This PR remedies the situation by watching for route changes in the
  Scope view, and reloading the new visualizer and sequence **into** the
  specimen already being viewed. Implementing this tactic requires the
  ability to completely change the sequence and visualizer of a specimen,
  which previously did not exist.

  The change also enables delaying specimen initialization from Vue setup()
  time to just before the component is mounted. As a result, the Vue setup
  is not asynchronous, so the experimental Vue <Suspense> tag is no longer
  needed.

  Adds a test that the back button actually took effect, to the existing
  test that changes a parameter. (The extended test definitely failed in
  ui2 prior to this PR.)

  Uses the router to redirect from an empty URL to the last-viewed specimen,
  rather than the Scope component; this change avoids reloading that specimen
  when the URL is updated to reflect what specimen is being viewed.

  Finally, fixes a small previously-existing bug in which the Differences
  visualizer could never display more terms than were available in its last
  parameter settings, even if you changed the parameters to make more
  terms available.

  Resolves numberscope#414.

  Given the large number of internal changes to Specimen.ts, please
  in review do play with a veriety of visualizations to make sure all seems
  to be working well.
@katestange
Copy link
Member

You said in the desc: "this change avoids reloading that specimen
when the URL is updated to reflect what specimen is being viewed"

Can you elaborate? If I'm looking at Thue Trellis and I delete the URL back to the base URL (http://localhost:5173), it does indeed restart the visualizer (the Thue Trellis turtle restarts the path).

@katestange
Copy link
Member

Ok there's definitely a performance issue on my machine. To reproduce, first try this in ui2 and then try it in this PR and see the difference:

(1) go to polyfactors, and scrub the mouse watching the immediate response of the mouseover highlight/info
(2) go to gallery and load vfib
(3) go to gallery and load thue trellis
(4) go to gallery and load wait for it
(5) go to gallery and load beatty dna
(6) return to polyfactors, and scrub the mouse watching the response. In this PR the response is laggy at this point.

Is it possible that some visualizers are not being destroyed as well as they used to be? So that the cpu is still trying to deal with all those webgl specimens?

@gwhitney
Copy link
Collaborator Author

Can you elaborate? If I'm looking at Thue Trellis and I delete the URL back to the base URL (http://localhost:5173), it does indeed restart the visualizer (the Thue Trellis turtle restarts the path).

Yes, I agree. The difference is that in ui2, when you visit the base URL, whatever the current query is gets loaded twice (in quick succession, so the first one may not be noticeable). Now it only happens once, because only one URL ever gets to the scope -- the one with the query filled in.

@gwhitney
Copy link
Collaborator Author

Is it possible that some visualizers are not being destroyed as well as they used to be? So that the cpu is still trying to deal with all those webgl specimens?

I agree something is up. Will investigate.

@gwhitney
Copy link
Collaborator Author

@katestange can you verify that the performance hit does not occur if rather than going through the Gallery, you just cut and paste the proper URLs into the address bar in turn?

I am pretty stymied to know what is going on here. Everything seems to be cleaning itself up in the scope. I can't get any draws that shouldn't be happening to console.log(), and if I add a message in the scope cleanup it seems always to be printed. So now I am wondering if it could be the Gallery itself that is causing the performance gap...

@gwhitney
Copy link
Collaborator Author

Trying to follow up on the idea that maybe the Gallery is the problem, Kate can you check whether if you start from a polyfactors that is responding OK, and then go to the Gallery and hit the back button (i.e., don't do a different scope), click on Gallery again and then do a back button, etc maybe 3 or 4 cycles, are you then seeing a performance hit when you get back to polyfactors after say 4 roundtrips? Thanks for letting me know. Have not had any success in tracking this down yet.

@katestange
Copy link
Member

Kate can you check whether if you start from a polyfactors that is responding OK, and then go to the Gallery and hit the back button (i.e., don't do a different scope), click on Gallery again and then do a back button, etc maybe 3 or 4 cycles, are you then seeing a performance hit when you get back to polyfactors after say 4 roundtrips?

Yes, I am definitely seeing a hit when I do this, but I see a similar hit doing this in ui2.

@katestange
Copy link
Member

@katestange can you verify that the performance hit does not occur if rather than going through the Gallery, you just cut and paste the proper URLs into the address bar in turn?

Yes, I can verify this.

@katestange
Copy link
Member

(1) go to polyfactors, and scrub the mouse watching the immediate response of the mouseover highlight/info
(2) go to gallery and load vfib
(3) go to gallery and load thue trellis
(4) go to gallery and load wait for it
(5) go to gallery and load beatty dna
(6) return to polyfactors, and scrub the mouse watching the response. In this PR the response is laggy at this point.

If I do all this except that I return to polyfactors by loading the URL instead of clicking the gallery icon, I don't have the performance problem.

@gwhitney
Copy link
Collaborator Author

except that I return to polyfactors by loading the URL instead of clicking the gallery icon, I don't have the performance problem.

Oh that makes it hard to debug because I guess changing the url manually in the url bar must be akin to doing a reload, which of course releases all the phantom visualizers (if indeed that is what's causing the performance issue). I will start by seeing if the Gallery seems to be leaving behind a mess, and if so, clean that up, and then take things from there.

@gwhitney
Copy link
Collaborator Author

Indeed, the Thumbnails in the Gallery were leaving behind a mess of visualizers, because their canvases were being removed from the DOM before the onUnmounted call was occurring, so the handle the code was using for the visualizer.depart() call was no longer valid at that time. Worked around this by saving a valid handle elsewhere.

And yes, this problem was present in ui2, but all of the reloading that was being triggered every time the specimen changed was masking it.

Correcting that leak of active visualizers fixes all the performance issues for me. You?

@katestange
Copy link
Member

Yes! The performance issues are gone!! I wonder if this will correct other performance issues elsewhere that we've come across. Excellent! I've tested this pretty extensively now, so I'm ready to merge.

@katestange katestange merged commit 5ab5bbf into numberscope:ui2 Oct 31, 2024
@gwhitney gwhitney deleted the back_button branch October 31, 2024 06:38
gwhitney added a commit that referenced this pull request Jan 20, 2025
* fix: Make sure browser 'back' button actually updates specimen

  Prior to this change, the browser 'back' button would always update the
  URL, but the visualization would typically not change if you were going
  from one scope URL (`/?name=Blah&...`) to another.

  This PR remedies the situation by watching for route changes in the
  Scope view, and reloading the new visualizer and sequence **into** the
  specimen already being viewed. Implementing this tactic requires the
  ability to completely change the sequence and visualizer of a specimen,
  which previously did not exist.

  The change also enables delaying specimen initialization from Vue setup()
  time to just before the component is mounted. As a result, the Vue setup
  is not asynchronous, so the experimental Vue <Suspense> tag is no longer
  needed.

  Adds a test that the back button actually took effect, to the existing
  test that changes a parameter. (The extended test definitely failed in
  ui2 prior to this PR.)

  Uses the router to redirect from an empty URL to the last-viewed specimen,
  rather than the Scope component; this change avoids reloading that specimen
  when the URL is updated to reflect what specimen is being viewed.

  Finally, fixes a small previously-existing bug in which the Differences
  visualizer could never display more terms than were available in its last
  parameter settings, even if you changed the parameters to make more
  terms available.

  Resolves #414.

  Given the large number of internal changes to Specimen.ts, please
  in review do play with a veriety of visualizations to make sure all seems
  to be working well.

* fix: Make sure Thumbnails clean up their visualizers
gwhitney added a commit that referenced this pull request Jan 12, 2026
* fix: Make sure browser 'back' button actually updates specimen

  Prior to this change, the browser 'back' button would always update the
  URL, but the visualization would typically not change if you were going
  from one scope URL (`/?name=Blah&...`) to another.

  This PR remedies the situation by watching for route changes in the
  Scope view, and reloading the new visualizer and sequence **into** the
  specimen already being viewed. Implementing this tactic requires the
  ability to completely change the sequence and visualizer of a specimen,
  which previously did not exist.

  The change also enables delaying specimen initialization from Vue setup()
  time to just before the component is mounted. As a result, the Vue setup
  is not asynchronous, so the experimental Vue <Suspense> tag is no longer
  needed.

  Adds a test that the back button actually took effect, to the existing
  test that changes a parameter. (The extended test definitely failed in
  ui2 prior to this PR.)

  Uses the router to redirect from an empty URL to the last-viewed specimen,
  rather than the Scope component; this change avoids reloading that specimen
  when the URL is updated to reflect what specimen is being viewed.

  Finally, fixes a small previously-existing bug in which the Differences
  visualizer could never display more terms than were available in its last
  parameter settings, even if you changed the parameters to make more
  terms available.

  Resolves #414.

  Given the large number of internal changes to Specimen.ts, please
  in review do play with a veriety of visualizations to make sure all seems
  to be working well.

* fix: Make sure Thumbnails clean up their visualizers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants