FactorHistogram overhaul (#568)

* fix: don't post popup when mouse button is down

* fix: correctly position labels when zooming

* refactor: move factoring/bin setup to presketch to get a jump on it

* fix: post warning about terms used for infinite sequence

* fix: don't block the browser when the number of terms is huge

* doc: explain hashing of first bar

* test: changes to ensure full suite of tests pass

* fix: show finer-grained progress on factoring long sequence

* fix: make sure the factor counts are recomputed when parameter change

* chore: reword hover box caption

* chore: remaining PR feedback, remove now-detrimental hover scaling

* fix: don't draw a y-axis tick mark that will get cut off; revert ci.yaml

Author: gwhitney

doc/visualizer-in-depth.md

@@ -298,6 +298,14 @@ fills it in from the `category`, and makes it read-only.
 
 ### Other properties
 
+#### Status of mouse primary button
+
+A P5Visualizer automatically maintains a property `mousePrimaryDown` that is
+true when the primary mouse button is in its pressed/down state. This property
+is in essence identical to `sketch.mouseIsPressed` but (a) it specifically
+only pays attention to the primary mouse button, and (b) has its value
+maintained more reliably in the face of events outside the sketch.
+
 #### Vue and reactive objects
 
 It is important to know that Vue will instrument (i.e., insert code into) your
@@ -390,17 +398,20 @@ opportunity to do pre-computation as well. That is the `presketch()` method,
 which runs asynchronously, meaning that the browser will not be blocked while
 this function completes. This facility is not a part of p5.js, but a part of
 the P5Visualizer design. The `presketch()` method is called by the framework
-with one argument, representing the size of the canvas to be created as a
-ViewSize object with number fields `width` and `height`.
+with two boolean arguments: the first specifies whether the sequence has
+changed since the last `presketch()`, and the second specifies whether the
+size has changed. (Both arguments are true for initialization.) You can obtain
+the size of the canvas to be created via the `this.size` property, a ViewSize
+object with number fields `width` and `height`.
 
 If you implement `presketch()`, begin by calling
-`await super.presketch(size)`, which will initialize the sequence that the
-visualizer is viewing. After this call, you have access to the values of the
-sequence, so you can do sequence-dependent initialization here. It is OK to
-set up internal data variables in this method. For example, this is a good
-place to populate an array with time-consuming precomputed values you will use
-repeatedly during the sketch. However, in `presketch()` you still have no
-access to the p5 canvas or the `this.sketch` object.
+`await super.presketch(seqChanged, sizeChanged)`, which will initialize the
+sequence that the visualizer is viewing. After this call, you have access to
+the values of the sequence, so you can do sequence-dependent initialization
+here. It is OK to set up internal data variables in this method. For example,
+this is a good place to populate an array with time-consuming precomputed
+values you will use repeatedly during the sketch. However, in `presketch()`
+you still have no access to the p5 canvas or the `this.sketch` object.
 
 Note also that `presketch()` is called when there is a new visualizer, when
 the sequence changes, when the canvas size changes, and when you reload the
@@ -409,6 +420,12 @@ parameters change. So if there is initialization you want to do only on these
 more signifcant changes but not on parameter changes, then `presketch()` is a
 good method.
 
+Note that since `presketch()` is called asynchronously, you cannot assume it
+has completed by the time any other method has been called, e.g. `setup()` or
+`draw()`. Therefore, P5Visualizer provides a boolean property
+`this.presketchComplete` that you can test to see if the presketch()
+initialization is done.
+
 When a visualizer is resized, or the restart button on Numberscope is pressed,
 the class function `reset()` is called. By default, a new canvas is created on
 a `reset()` (and so `setup()` will be called). In this event, `presketch()`

e2e/tests/featured.spec.ts

@@ -9,13 +9,13 @@ test.describe('Featured gallery images', () => {
         const featProps = parseSpecimenQuery(feature.query)
         const details = {}
         if (
-            featProps.visualizerKind === 'Histogram'
+            featProps.visualizerKind === 'FactorHistogram'
             || featProps.visualizerKind === 'Turtle'
             || featProps.visualizerKind === 'Chaos'
         ) {
             details.tag = '@webGL'
         }
-        test(featProps.name, details, async ({page, browserName}) => {
+        test(featProps.name, details, async ({page}) => {
             const short = encodeURIComponent(
                 featProps.name.replaceAll(' ', '')
             )
@@ -27,9 +27,7 @@ test.describe('Featured gallery images', () => {
                 timeout: featProps.visualizerKind === 'Chaos' ? 60000 : 30000,
             })
             const matchParams =
-                browserName === 'firefox' && details.tag === '@webGL'
-                    ? {maxDiffPixelRatio: 0.01}
-                    : {}
+                details.tag === '@webGL' ? {maxDiffPixelRatio: 0.01} : {}
             expect(
                 await page.locator('#canvas-container').screenshot()
             ).toMatchSnapshot(`${short}.png`, matchParams)

e2e/tests/transversal.spec.ts

@@ -69,7 +69,11 @@ test.describe('Visualizer-sequence challenges', () => {
     for (const viz of vizKeys) {
         const vizPar = viz === 'Chaos' ? 'circSize=5' : '' // ow tough to see
         const details = {}
-        if (viz === 'Histogram' || viz === 'Turtle' || viz === 'Chaos') {
+        if (
+            viz === 'FactorHistogram'
+            || viz === 'Turtle'
+            || viz === 'Chaos'
+        ) {
             details.tag = '@webGL'
         }
         for (const seq of vizSeqs[viz]) {

e2e/tests/transversal.spec.ts-snapshots/FactorHistogramA001220-chromium-linux.png

@@ -77,7 +77,7 @@ click on the trash button on its preview card.
         addSequence,
         deleteSequence,
     } from '@/shared/browserCaching'
-    import {isMobile} from '@/shared/layoutUtilities'
+    import {isMobile} from '@/shared/layout'
     import {Specimen} from '@/shared/Specimen'
     import {
         specimenQuery,

e2e/tests/transversal.spec.ts-snapshots/FactorHistogramA001220-firefox-linux.png

@@ -1,6 +1,7 @@
 import type {Factorization, SequenceInterface} from './SequenceInterface'
 import simpleFactor from './simpleFactor'
 
+import {yieldExecution} from '@/shared/asynchronous'
 import {math, CachingError} from '@/shared/math'
 import type {ExtendedBigint} from '@/shared/math'
 import {Paramable, paramClone} from '@/shared/Paramable'
@@ -151,7 +152,7 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
         firstValueCached: ExtendedBigint = math.posInfinity
         lastFactorCached = -1024n // dummy value
         firstFactorCached: ExtendedBigint = math.posInfinity
-        valueCachingPromise = Promise.resolve()
+        valueCachingPromise: Promise<void> | undefined = undefined
         factorCachingPromise = Promise.resolve()
 
         /**
@@ -256,6 +257,7 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
         async fillValueCache(n: bigint) {
             const start = this.lastValueCached + 1n
             for (let i = start; i <= n; i++) {
+                if (i % 10000n === 0n) await yieldExecution()
                 const key = i.toString()
                 // trust values we find; hopefully we have cleared when
                 // needed, so that we can presume they are from some
@@ -269,16 +271,13 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
         }
 
         async cacheValues(n: bigint) {
-            // Let any existing value caching complete
-            await this.valueCachingPromise
-            // Let any pending parameter changes complete
             if (this.parChangePromise) await this.parChangePromise
+            if (this.valueCachingPromise) await this.valueCachingPromise
             if (n > this.lastValueCached) {
                 this.valueCachingPromise = this.fillValueCache(n)
                 await this.valueCachingPromise
-            } else {
-                this.valueCachingPromise = Promise.resolve()
             }
+            this.valueCachingPromise = undefined
         }
 
         getElement(n: bigint): bigint {
@@ -299,6 +298,8 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
             await this.cacheValues(n)
             const start = this.lastFactorCached + 1n
             for (let i = start; i <= n; ++i) {
+                // Can we yield execution?
+                if (i % 10000n === 0n) await yieldExecution()
                 const key = i.toString()
                 // trust values we find
                 if (!(key in this.factorCache)) {
@@ -393,10 +394,13 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
                     // interval from the _previous_ value of `this.first`
                     // to `this.lastValueCached` is full.
                     if (
-                        this.first < this.firstValueCached
-                        || this.first > this.lastValueCached + 1n
+                        !needsReset
+                        && (this.first < this.firstValueCached
+                            || this.first > this.lastValueCached + 1n)
                     ) {
-                        await this.valueCachingPromise
+                        if (this.valueCachingPromise) {
+                            await this.valueCachingPromise
+                        }
                         this.firstValueCached = math.posInfinity
                         this.lastValueCached = this.first - 1n
                         // Get the new cache rolling:
@@ -406,8 +410,9 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
                     // kick off the factoring process because we don't
                     // even know if this sequence is being factored.
                     if (
-                        this.first < this.firstFactorCached
-                        || this.first > this.lastFactorCached + 1n
+                        !needsReset
+                        && (this.first < this.firstFactorCached
+                            || this.first > this.lastFactorCached + 1n)
                     ) {
                         await this.factorCachingPromise
                         this.firstFactorCached = math.posInfinity
@@ -441,7 +446,9 @@ export function Cached<PD extends GenericParamDescription>(desc: PD) {
             }
             if (needsReset) {
                 this.ready = false
-                await this.valueCachingPromise
+                if (this.valueCachingPromise) {
+                    await this.valueCachingPromise
+                }
                 await this.factorCachingPromise
                 this.initialize()
             }

e2e/tests/transversal.spec.ts-snapshots/FactorHistogramA002819-chromium-linux.png

@@ -5,7 +5,7 @@ import type {Factorization} from './SequenceInterface'
 import simpleFactor from './simpleFactor'
 
 import {alertMessage} from '@/shared/alertMessage'
-import {breakableString} from '@/shared/layoutUtilities'
+import {breakableString} from '@/shared/layout'
 import {math} from '@/shared/math'
 import type {ExtendedBigint} from '@/shared/math'
 import type {GenericParamDescription} from '@/shared/Paramable'