chore: wip attempt to upgrade deck&luma

[xinaesthete]

Addresses # https://github.com/hms-dbmi/viv/issues/804

Background

As discussed https://github.com/hms-dbmi/viv/discussions/790

Fairly extensive changes will be required; discussion should probably migrate to here.

[ilan-gold]

I made some fixes (commits should be messaged clearly). Thanks for getting this started. More to come tomorrow!

I am not clear what was previously dataFormat should be now, but that might not be what is causing the current error:

deck: drawing XRLayer({id: 'image-sub-layer-0,135,97,0-Background-Image-ZarrPixelSource-#detail#'}) to screen: cyclic object value TypeError: cyclic object value
    setUniformsWebGL webgl-render-pipeline.js:197
    setUniformsWebGL webgl-render-pipeline.js:196
    setUniforms model.js:464
    draw xr-layer.js:257
    _drawLayer layer.js:851
    withGLParameters with-parameters.js:17
    withParametersWebGL webgl-device.js:269
    _drawLayer layer.js:845
    _drawLayersInViewport layers-pass.js:153
    _drawLayers layers-pass.js:54
    render layers-pass.js:32
    renderLayers deck-renderer.js:47
    _drawLayers deck.js:691
    redrawDeck deckgl.js:42
    _customRender deckgl.js:65
    redraw deck.js:369
    _onRenderFrame deck.js:723
    _renderFrame animation-loop.js:256
    redraw animation-loop.js:154
    _animationFrame animation-loop.js:244
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    _animationFrame animation-loop.js:245
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    _animationFrame animation-loop.js:245
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    _animationFrame animation-loop.js:245
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    _animationFrame animation-loop.js:245
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    _animationFrame animation-loop.js:245
    requestAnimationFrame request-animation-frame.js:9
    _requestAnimationFrame animation-loop.js:225
    start animation-loop.js:119
    _Deck deck.js:251
    createDeckInstance deckgl.js:47
    DeckGLWithRef deckgl.js:138
    React 8
    workLoop scheduler.development.js:266
    flushWork scheduler.development.js:239
    performWorkUntilDeadline scheduler.development.js:533
    scheduler chunk-KSA3GRLC.js:405
    scheduler chunk-KSA3GRLC.js:453
    __require2 chunk-LK32TJAX.js:18
    scheduler chunk-KSA3GRLC.js:465
    __require2 chunk-LK32TJAX.js:18
    React 2
    __require2 chunk-LK32TJAX.js:18
    dom React
    __require2 chunk-LK32TJAX.js:18
    dom React
    __require2 chunk-LK32TJAX.js:18
    <anonymous> react-dom_client.js:38

[xinaesthete]

Yeah - as for clear messaging of commits, I'm generally very on-board with that, previous commit was a result of starting to mess around & seeing how far I got, then figured it was more useful to commit than just leave on my machine...

I see that as far up as model.setUniforms() is marked as deprecated, so that may be the point at which to change the code for the current issue.

[xinaesthete]

https://github.com/visgl/luma.gl/blob/master/docs/legacy/porting-guide.md#quick-v9-api-overview

In progress Reading and writing buffers is now an async operation. While WebGL does not support async reads and writes on MacOS, the API is still async to ensure portability to WebGPU. Uniform buffers are now the standard way for the application to specify uniforms. Uniform buffers are "emulated" under WebGL.

[xinaesthete]

I think that what's happening is that likely the dataFormat is indeed causing it to raise an error, and then in the process of raising it it gets circular references because the texture objects refer back to the device...

I think setting uniforms like that should work in the short-term although it'll clearly need changing once it gets to making it run on WebGPU etc.

[xinaesthete]

https://luma.gl/docs/api-reference/core/texture-formats/ (not sure how helpful I'm being here, just dropping things in that seem relevant)

Note that luma.gl deduces dataFormat and type from format by taking the first value from the data format and data type entries in this table.

[xinaesthete]

I optimistically tried changing draw() in xr-layer.js:256

      model
        .setBindings({uniforms: {
          ...uniforms,
          contrastLimits: paddedContrastLimits,
          ...textures
        }})
      model.draw(); //n.b. took a few moments to realise setBindings() returns undefined...

I've not really processed whether that's actually a correct way to be passing uniforms, but it does change the kind of error we see...

webgl-render-pipeline.js:243 Uncaught (in promise) 
Error: Error during linking: Vertex shader is not compiled.

    at WEBGLRenderPipeline._reportLinkStatus (webgl-render-pipeline.js:243:23)
    at WEBGLRenderPipeline._linkShaders (webgl-render-pipeline.js:218:18)
    at new WEBGLRenderPipeline (webgl-render-pipeline.js:54:14)
    at _WebGLDevice.createRenderPipeline (webgl-device.js:226:16)
    at _PipelineFactory.createRenderPipeline (pipeline-factory.js:29:42)
    at _Model._updatePipeline (model.js:551:50)
    at new _Model (model.js:160:30)
    at XRLayer._getModel (xr-layer.js:189:12)
    at XRLayer.updateState (xr-layer.js:159:35)
    at XRLayer._update (layer.js:778:22)

I don't know how important the warnings that appear before that (luma.gl: Model(image-sub-layer-0,131,175,0-Background-Image-TiffPixelSource-#detail#): Missing layout for buffer "instancePickingColors". etc) are.

Actually, the error thrown in _reportLinkStatus() happens before we call setUniforms/Bindings() - it's just that when we call setUniforms() then it raises another error in the process, which makes it onto the console before the other error is reported, whereas when we call setBindings() it appears happier, then in the process of drawing picks up on the link status & outputs the earlier error. Then having printed that error once, the app is running moderately happily and it'll draw the scale-bar layers (with some one-frame-behind annoyingness, at least with devtools open, I've seen that behave differently based on those kinds of conditions at times in MDV) - so it's passing through the XRLayer.draw() without incident, updating the app state, just not managing to draw anything particularly useful.

This is where things get flagged in lumagl webgl-render-pipeline.js

    // PRIVATE METHODS
    // setAttributes(attributes: Record<string, Buffer>): void {}
    // setBindings(bindings: Record<string, Binding>): void {}
    async _linkShaders() {
        const { gl } = this.device;
        gl.attachShader(this.handle, this.vs.handle);
        gl.attachShader(this.handle, this.fs.handle);
        log.time(LOG_PROGRAM_PERF_PRIORITY, `linkProgram for ${this.id}`)();
        gl.linkProgram(this.handle);
        log.timeEnd(LOG_PROGRAM_PERF_PRIORITY, `linkProgram for ${this.id}`)();
        // TODO Avoid checking program linking error in production
        if (log.level === 0) {
            // return;
        }
        if (!this.device.features.has('compilation-status-async-webgl')) {
            const status = this._getLinkStatus();
            this._reportLinkStatus(status); //<<<<<<<
            return;
        }
        // async case
        log.once(1, 'RenderPipeline linking is asynchronous')();
        await this._waitForLinkComplete();
        log.info(2, `RenderPipeline ${this.id} - async linking complete: ${this.linkStatus}`)();
        const status = this._getLinkStatus();
        this._reportLinkStatus(status);
    }

FWIW on my M1 running Chrome stable in Sonoma, it doesn't have the ''compilation-status-async-webgl' feature. Hopefully we're fairly close to the point of seeing why it hasn't linked at this stage... both fs and vs of the WebGLRenderPipeline in question have compilationStatus: "pending".

I might commit my minor change without great confidence in its correctness... just going to dig through the debugger & docs a bit more and see if I make any more progress...

[xinaesthete]

ok... so a couple of things of note.

When I changed const programManager = ProgramManager.getDefaultProgramManager(device) to ShaderAssembler.getDefaultShaderAssembler(), at no point did I make any change meaning that the assembleShader() method was called on it - indeed, that seems not to be happening at present, and it seems likely this may be needed before getting to shader linking.

I could do with better understanding how pipelines are managed - bearing in mind that PipelineManager is the thing that's supposed to replace ProgramManager, I guess although changing to ShaderAssembler was enough to pass a basic build step, there may be a need to involve PipelineManager... I'm also not entirely clear on how attributes are supposed to work... in AttributeManager vs Geometry, and the new bufferLayout prop for Model... So I think I need to a) take a rest, and b) read through more of the docs.

When we call new Model(...), it creates a pipeline with its pipelineFactory, which is PipelineFactory.getDefaultPipelineFactory(this.device). That is used to make a ShaderFactory (not ShaderAssembler), which is then used in Model._updatePipeline():

    /** Update pipeline if needed */
    _updatePipeline() {
        if (this._pipelineNeedsUpdate) {
            let prevShaderVs = null;
            let prevShaderFs = null;
            if (this.pipeline) {
                log.log(1, `Model ${this.id}: Recreating pipeline because "${this._pipelineNeedsUpdate}".`)();
                prevShaderVs = this.pipeline.vs;
                prevShaderFs = this.pipeline.fs;
            }
            this._pipelineNeedsUpdate = false;
            const vs = this.shaderFactory.createShader({
                id: `${this.id}-vertex`,
                stage: 'vertex',
                source: this.source || this.vs,
                debug: this.props.debugShaders
            });
            let fs = null;
            if (this.source) {
                fs = vs;
            }
            else if (this.fs) {
                fs = this.shaderFactory.createShader({
                    id: `${this.id}-fragment`,
                    stage: 'fragment',
                    source: this.source || this.fs,
                    debug: this.props.debugShaders
                });
            }
            this.pipeline = this.pipelineFactory.createRenderPipeline({
                ...this.props,
                bufferLayout: this.bufferLayout,
                topology: this.topology,
                parameters: this.parameters,
                vs,
                fs
            });
            this._attributeInfos = getAttributeInfosFromLayouts(this.pipeline.shaderLayout, this.bufferLayout);
            if (prevShaderVs)
                this.shaderFactory.release(prevShaderVs);
            if (prevShaderFs)
                this.shaderFactory.release(prevShaderFs);
        }
        return this.pipeline;
    }

When that gets into this.pipelineFactory.createRenderPipeline(...), it has props including shaderAssembler which is indeed a reference to getShaderAssembler(device) with relevant hooks added... when it eventually gets in to new WEBGLRenderPipeline() and _linkShaders(), as noted before, the shaders have compilationStatus: "pending" and I'm not sure on what basis they'd be compiled by then. But maybe my problem is that I've spent more time stepping through code than actually reading the docs...

I notice that the _hookFunctions property is meant to be private... I wonder if there might be somewhere else to add these hooks rather than initializeState(), but that's a relatively minor concern - I don't think it'd stop anything working.

Anyway, I suppose I'll push what I have currently - not confident it's 100% correct.

[xinaesthete]

OK... I think I've made relevant shader edits. Now I get

deck: drawing XRLayer({id: 'image-sub-layer-0,131,175,0-Background-Image-TiffPixelSource-#detail#'}) to screen: No value for binding channel0 in image-sub-layer-0,131,175,0-Background-Image-TiffPixelSource-#detail#-cached Error: No value for binding channel0 in image-sub-layer-0,131,175,0-Background-Image-TiffPixelSource-#detail#-cached
    at WEBGLRenderPipeline._applyBindings (webgl-render-pipeline.js:314:23)
    at WEBGLRenderPipeline.draw (webgl-render-pipeline.js:167:14)
    at _Model.draw (model.js:256:41)
    at XRLayer.draw (xr-layer.js:278:13)
    at layer.js:851:22
    at withGLParameters (with-parameters.js:17:16)
    at _WebGLDevice.withParametersWebGL (webgl-device.js:269:16)
    at XRLayer._drawLayer (layer.js:845:28)
    at DrawLayersPass._drawLayersInViewport (layers-pass.js:153:27)
    at DrawLayersPass._drawLayers (layers-pass.js:54:36)

I suspect we need to work on how the uniform buffers are arranged.

The bindings of the WebGLRenderPipeline that throws the error is looking for 'channel0' etc, but there is only key, 'pickingUniforms', associated with a WEBGLBuffer.

Putting a breakpoint on setBindings(), I see that the image layer is calling it with WEBGLBuffers for various modules, plus our plain js object 'uniforms':

[xinaesthete]

OK, I think we're getting close: I actually see some graphics getting drawn, albeit not very useful.

I think maybe the out fragColor just isn't bound to an actual framebuffer; if I put fragColor.r = 1.0; at the end of the fs it does nothing. more like the geometry indices / texCoords etc are dodgy - toggling channels changes colors! Exciting times!

[xinaesthete]

It's starting to look like image data... something bad with some constants / parameters for setting sampler attribs or something I guess...

[xinaesthete]

According to https://luma.gl/docs/api-reference/core/texture-formats/ 'r32float' is not filterable (rings unpleasant bells)

[xinaesthete]

Pretty sure it's the GL.(UN)PACK_ALIGNMENT that's an issue now.

[xinaesthete]

Seems contrastLimits uniforms somehow aren't getting through... odd that colors are, not much different about the format.

[xinaesthete]

phew

Not finished, but now rendering non-glitchy images.

[ilan-gold]

According to https://luma.gl/docs/api-reference/core/texture-formats/ 'r32float' is not filterable (rings unpleasant bells)

What's weird about this is that it appears to have been working before. If you go to this image on the Avivator website, it does linear filtering on 32 bit data. I think there's an extension for this.

Edit: it's OES_texture_float_linear

[ilan-gold]

~I think this may be coming from luma.gl somehow actually...~ sorry this is something you wrote

[ilan-gold]

Very cursed, if you just don't check the filterable-ness, you can just use linear.

[ilan-gold]

@xinaesthete Can you try out the float32 image? It is kind of slow for me, but that might just be the fact that I have a bunch of stuff open at once. If it's jumpy, then we might have an issue. There also might be a bug in luma.gl because running

    console.log(this.context.device.features.has('float32-filterable'))

    if (!this.context.device.isTextureFormatFilterable(attrs.format)) {
      attrs.filter = 'nearest';
      console.warn(`Texture format ${attrs.format} is not filterable. Falling back to 'nearest' interpolation.`);
    }

prints both True for the feature and then also warns which means the extension is somehow not being discovered properly?

[xinaesthete]

I've been mostly (exclusively, so far) testing with the default image https://viv-demo.storage.googleapis.com/LuCa-att7color_3x3component_data.ome.tif which is using r32float for the texture format, and don't notice any performance issue on my M1Pro@4k, with a few things going on / quite a few screens / etc.

I agree that it does indeed appear to render with the filter. I wonder if they say this format isn't filterable because it's different in WebGPU or something? I have a feeling I may have run into that in WebGPU.

Somewhat relatedly, I have a bug in MDV where there's a 'pop-out chart' button that moves a div to a new window. When I do this with a viv viewer (normal npm version), I get some spurious errors like deck: initialization of XRLayer({id: 'image-sub-layer-0,178,178,0-Background-Image-TiffPixelSource-#EXcsTUdetail-react#'}): WebGL1 context does not support floating point textures. Unable to display raster data.. In that context the gl object is a WebGL2RenderingContext with _version: 1... I'm also rendering a GeoJsonLayer on top which gets weirdly glitchy vertex corruption, along with a ScatterplotLayer (and the viv scalebars/text)... Aside from viv giving up on the images and the weird GeoJson, everything looks normal. I don't think it loses the GL context in the process of the transition etc, not sure why that _version changes - but I'm very much hoping this issue will go away once we get this upgrade through.

edit: in the popout issue I mentioned, it renders ok again when popped back in, and _version returns to 2.

[ilan-gold]

I've been mostly (exclusively, so far) testing with the default image https://viv-demo.storage.googleapis.com/LuCa-att7color_3x3component_data.ome.tif which is using r32float for the texture format, and don't notice any performance issue on my M1Pro@4k, with a few things going on / quite a few screens / etc.

I agree that it does indeed appear to render with the filter. I wonder if they say this format isn't filterable because it's different in WebGPU or something? I have a feeling I may have run into that in WebGPU.

including my latest push? then i probably have too much on my computer. so that's good.

[ilan-gold]

Somewhat relatedly, I have a bug in MDV where there's a 'pop-out chart' button that moves a div to a new window. When I do this with a viv viewer (normal npm version), I get some spurious errors like deck: initialization of XRLayer({id: 'image-sub-layer-0,178,178,0-Background-Image-TiffPixelSource-#EXcsTUdetail-react#'}): WebGL1 context does not support floating point textures. Unable to display raster data.. In that context the gl object is a WebGL2RenderingContext with _version: 1... I'm also rendering a GeoJsonLayer on top which gets weirdly glitchy vertex corruption, along with a ScatterplotLayer (and the viv scalebars/text)... Aside from viv giving up on the images and the weird GeoJson, everything looks normal. I don't think it loses the GL context in the process of the transition etc, not sure why that _version changes - but I'm very much hoping this issue will go away once we get this upgrade through.

edit: in the popout issue I mentioned, it renders ok again when popped back in, and _version returns to 2.

Can you open an issue with a reproducer?

[xinaesthete]

including my latest push? then i probably have too much on my computer. so that's good.

Yeah I can't think of any reasons to believe there's any particular regression - should be easy enough for you try to another version on your computer & see how that goes.

I'll think about what the most effective way for me to make a repro for that issue is, I should be able to put some kind of test project somewhere accessible to you. As I say, it's related in that it's to do with feature detection - and I think maybe the corruption of the geojson layer could be to do with not properly resetting state when the error occurs, but I haven't looked too deeply.

[xinaesthete]

There's a problem with volumetric data. It seems like a bug in luma.gl to me; I'll report it there and link back to here for repro. In xr-3d-layer.js, we call this (I believe that it is now not necessary to pass anything other than the format as a string, ie. 'r32float'):

    const attrs = getRenderingAttrs(); // attrs.format = 'r32float'
    const texture = this.context.device.createTexture({
      width,
      height,
      depth,
      dimension: '3d',
      data: attrs.cast?.(data) ?? data,
      format: attrs.format,
      mipmaps: false,
      sampler: {
        minFilter: 'linear',
        magFilterFilter: 'linear',
        addressModeU: 'clamp-to-edge',
        addressModeV: 'clamp-to-edge',
        addressModeW: 'clamp-to-edge'
      }
    });

luma.gl in the process of initialising the texture ends up calling convertTextureFormatToGL with format == 33326, having already at an earlier stage in the process converted from string format to GL constant. So it throws an Unsupported texture format error. I briefly tried to patch my local version of luma to see how far it'd get if I overrode that, but didn't manage to apply that correctly.

[ilan-gold]

Thanks @xinaesthete I was looking into the scale bar issue, why it's all jumpy. I think the hack I had working for a while is over so I'll need to make a breaking change there as well. We should probably release that along with this at the same time.

[xinaesthete]

Thanks @xinaesthete I was looking into the scale bar issue, why it's all jumpy. I think the hack I had working for a while is over so I'll need to make a breaking change there as well. We should probably release that along with this at the same time.

I have a weird thing in MDV, or I did at one point, where the scale-bar was jumpy only with devtools open... which is not the case now, and irritatingly I don't know what changed to fix it (I'm very conscious that it's something that should definitely be avoided).

I definitely get a bit confused about the whole deck.gl/react render cycle sometimes... actually, it might be worth thinking about updating React/MaterialUI in Avivator - I had partially done this in another branch locally, along with the deck upgrade... obviously wasn't in a clean state for pushing with so many changes, it was mostly for experimentation, and with actual viv graphics not working it was hard to be confident about whether the UI stuff was ok...

I guess the scale-bar and deck update cycle should be separate from React, but I'd say it's definitely worth testing in a non-Avivator context as well (which I haven't done so far with this branch) and evaluating whether it may be time to put in that work... New React compiler etc is also looking worth updating to.

FWIW I was also at one point (and may do again) rendering DOM elements over the canvas, with transforms that I tried to keep in sync, and had similar issues with being one-frame-behind and also not having the right design so I'd end up with frames where I'd try to project/unproject with a deck object that wasn't 'ready' / null-reference etc. So it'd definitely be good to have a more proper understanding of this stuff.

Final side-note on the topic of the scale-bar... I end up with it being occluded by other layers and I'm not sure what I'd do to change that from the public interface (we could probably change it to be rendered last with no depth-test in viv, though).

[xinaesthete]

Just looked again, and scale-bar is jumpy in Chrome for this version only when devtools is open. In Safari, it's dodgy regardless of devtools - and actually, the zooming feels glitchy there as well.

Same for production Avivator site... but in MDV, where I'm using the same scale-bar, it's not.

So... one hypothesis is that it could indeed be related to the react version - that's one thing that is different... but there are lots of other differences. I think that's a likely one, though. edit: could also be related to different zustand version.

[ilan-gold]

one hypothesis is that it could indeed be related to the react version - that's one thing that is different

Looks like https://github.com/Taylor-CCB-Group/MDV/blob/93a50ee5a7f62bd1cf395669885422cc4eb3784c/package.json#L33-L56 is similar to Avivator: https://github.com/hms-dbmi/viv/blob/main/sites/avivator/package.json

I think it's a state management thing. Could you point to zustand? I didn't see it in you repo.

[xinaesthete]

Main branch is very stale, but https://github.com/Taylor-CCB-Group/MDV/tree/pjt-dev/src/react/components/avivatorish is adapted version of Avivator functionality (quite a bit of copy/paste where I'd been thinking in terms of avivator being made into an actual library)...

It's a bit more strongly-typed than Avivator, but not fully.

[xinaesthete]

So yeah, https://github.com/Taylor-CCB-Group/MDV/blob/pjt-dev/package.json for versions (zustand 4.4.5, have a feeling that could updated actually).

[xinaesthete]

n.b. if you want to actually run it, apologies if state of documentation etc bears something to be desired. Will hopefully get things straightened out before too long.

[xinaesthete]

I just tried my viv branch from this PR with more recent Zustand & some changes to how it's called, and the scale-bar appears not to be glitchy. edit: also behaves better in Safari.

[ilan-gold]

@xinaesthete If you're still interested in that PR, I can have another look and merge soon. I apologize for not being more responsive, but up until the past week or so, this wasn't part of my daily work. But it's in the rotation now :)

[xinaesthete]

@xinaesthete If you're still interested in that PR, I can have another look and merge soon. I apologize for not being more responsive, but up until the past week or so, this wasn't part of my daily work. But it's in the rotation now :)

Thanks, I think there's a legitimate debate as to whether that PR itself should merge - whether you want to support "Avivator as a library" etc, otherwise the context-related changes are kinda noise in the code... It won't be a lot of work to redo the Zustand update without the context noise.

My "avivatorish" code is starting to diverge somewhat - treating the original as a demo / starting-off point, I was somewhat procrastinating about how much to diverge from the original within the bits that were originally copied. For instance, I just implemented features for loading the saved channels/view state (essentially, in useImage, I changeLoader().then(applyConfig), but this is likely to need to cut a bit deeper over time).

I am currently fairly comfortable with the idea that I treat it as a separate code-base that happens to strongly based off Avivator (and dependent on Viv), and will likely diverge further in terms of UI and various other aspects more in future... Perhaps we could arrange a call sometime if that suits you?

[ilan-gold]

It won't be a lot of work to redo the Zustand update without the context noise.

On that now.

I am currently fairly comfortable with the idea that I treat it as a separate code-base that happens to strongly based off Avivator (and dependent on Viv), and will likely diverge further in terms of UI and various other aspects more in future... Perhaps we could arrange a call sometime if that suits you?

I would be open to this. Always open to ideas. ilanbassgold@gmail.com

My "avivatorish" code is starting to diverge somewhat - treating the original as a demo / starting-off point, I was somewhat procrastinating about how much to diverge from the original within the bits that were originally copied. For instance, I just implemented features for loading the saved channels/view state (essentially, in useImage, I changeLoader().then(applyConfig), but this is likely to need to cut a bit deeper over time)....will likely diverge further in terms of UI and various other aspects more in future... Perhaps we could arrange a call sometime if that suits you?

I suspect this is why Trevor in all his infinite wisdom never wanted to make Avivator a package. You end up eventually wanting more and more.

[xinaesthete]

It won't be a lot of work to redo the Zustand update without the context noise.

On that now.

Cool. I think it should fix the glitchiness of zoom etc.

I suspect this is why Trevor in all his infinite wisdom never wanted to make Avivator a package. You end up eventually wanting more and more.

Yeah, as I say, I'd have developed things differently if it was, but think it's reasonable to keep it as is and don't disagree with Trevor's assessment...

I'll chat with my boss & maybe try to set something up soon. Hopefully there'll be some response on the 3D texture issue as well at some point.

[ilan-gold]

Cool. I think it should fix the glitchiness of zoom etc.

It unfortunately did not but I did discover that the problem is only present on FF which makes sense given our issues with it in the past around higher-performance CPU stuff.

[ilan-gold]

The only way I can sort of reproduce the scale bar wonkiness is by using the lens in google chrome (on the main site as well). Zooming in and out seems sketchy. So I'd venture a guess there is something going on with state management and rendering times. Both the lens and scalebar take the current view state and render something based on a bounding box from it (in the lens' case, that is an ellipse which is rescaled to a circle on the GPU).

[ilan-gold]

But I'd say the scalebar wonkiness is independent of this so I will drop it for now here.

[xinaesthete]

Cool. I think it should fix the glitchiness of zoom etc.

It unfortunately did not but I did discover that the problem is only present on FF which makes sense given our issues with it in the past around higher-performance CPU stuff.

I think that as well as updating viv & refactoring to use context, there was another change I made to do with eqFn that I think may be the relevant factor here - something that was flagged as a warning somewhere IIRC. I'll have a go at this myself - I can probably more easily parse the relevant changes in my old PR - and push to here if I think it's worked.

[xinaesthete]

ok so I just realised I don't even see scale-bar at all on Firefox - hadn't been checking it while there were still a lot of other issues to clear up. It is ok on the production site (and I could only coax very minor glitches out of it when zooming fast), but it seems to not be visible with my context branch or this one.

Performance etc seem ok there (although I've certainly seen it be much worse than others in the past - I was experimenting with pyodide and actually iirc Safari was 2x faster than Chrome, FF ~4x slower).

[xinaesthete]

So the thing I was thinking of in Zustand was that import shallow from 'zustand/shallow' said

@deprecated — Use import { shallow } from 'zustand/shallow'

So that is a very quick find/replace job... It doesn't seem to make an appreciable difference, though... I think I'll push it here though because it's a small change, and at least fits the topic of updating dependencies / fairly sure not doing any harm.

I've now realised now there is a separate bug that was causing the scale-bar not to show, and it happens consistently with https://viv-demo.storage.googleapis.com/LuCa-7color_Scan1/ across browsers - must be different for Zarr vs OME-TIFF. So that should be filed as a separate issue & dealt with outside this PR.

[xinaesthete]

I've been trying to see how we're doing with the tests... I'm not as familiar with how they work as I'd like to be, right now it seems like there are things that just hang indefinitely rather than showing the actual problem.

If I do this in packages/layers/tests/index.spec.js, then everything else passes.

// import './xr-layer.spec';
// import './multiscale-image-layer-base.spec';
// import './scale-bar-layer.spec';

Last output from each of those before it hangs:

• XRLayer#interpolation: nearest1

• MultiscaleImageLayerBase#pickable: false

  ✔ should be truthy

• ScaleBarLayer#Empty props

Not clear to me how to relate this information to what needs to be fixed.

p.s. it looks like we don't currently have tests for xr-3d-layer - which is the main thing I'm aware of currently being definitely broken. No response from luma.gl on the upstream bug there, I could probably have made the issue more useful for them, not sure how much effort it is to make a minimal repro for 3D textures.

[xinaesthete]

OK, based on my testing locally, the glitchy scale-bar (and generally glitchy zoom in Safari) appear to be fixed by my context branch.

I guess it must be to do with

useViewerStore.setState({...})

vs

const viewerStore = useViewerStoreApi();
useEffect(() => {
  viewerStore.setState({...})
})

Where useEffect is anything that involves setting zustand state outside the react component rendering (i.e. mouse callback etc). It probably only matters for one or two points of the code (zoom being the main one).

This is really a separate issue to what this PR is supposed to be for, not a big hassle to make a separate PR for it if you like.

useXXXStoreApi et al was my convention for this - could probably be applied very selectively to deal with this specific issue.

Really, seems like maybe a bit of a zustand issue?

[xinaesthete]

I haven't been able to reproduce the 3D texture bug by modifying the luma.gl example to use 'r32float', so need to do a bit more work to figure out what is different about our circumstances that cause it to attempt to double-convert from string to GL constant.

[xinaesthete]

https://github.com/visgl/luma.gl/pull/2123 is relevant to this.

Some texture examples (texture-3d, cubemap, ...) are non-trivially broken

Our issue with 3d texture creation seems to be fixed in luma main even before that PR (and their tests seem to pass as well), but presumably there are more things that they know to be broken internally.

[ilan-gold]

@xinaesthete If you want to point this branch at main there, I'd be fine with that. We can't release anyway. I just discovered that rbg8norm-webgl isn't available anyway, and we rely on that. I'll need to open an issue and pray the solutions is not that we need to add an extra channel

[xinaesthete]

@xinaesthete If you want to point this branch at main there, I'd be fine with that.

ok I might try that, would be nice to at least see volumetric data working.

We can't release anyway. I just discovered that rbg8norm-webgl isn't available anyway, and we rely on that. I'll need to open an issue and pray the solutions is not that we need to add an extra channel

That would be for an image with 8bit RGB data I assume? I need to figure out which image to use to test that, might need to generate one - I've just been going through your examples in source-info.js and adding comments on the channel configuration. It'd be good to have something with t !== 1 for testing as well.

[ilan-gold]

That would be for an image with 8bit RGB data I assume? I need to figure out which image to use to test that, might need to generate one - I've just been going through your examples in source-info.js and adding comments on the channel configuration. It'd be good to have something with t !== 1 for testing as well.

I have one locally but I'm not sure it's public. I could possibly send to you via email.

[xinaesthete]

New luma.gl version, doesn't fix the Texture-3D issue which I optimistically expected it would.

Also not sure what's up with the Vercel workflow failing here.

[xinaesthete]

Hopefully there'll be a luma.gl release that'll be usable soon - I'll get back to this when I can (although I will be away for a few days so maybe not totally immediately).

It might be good to also update React / Material-UI / Zustand; currently in MDV we get warnings about React version but no known actual issues. I could potentially have a go at that as well (actually I think I previously did most of that work in another branch when I started on this before). In some ways it'd be simpler to do it within this PR but also might be better to separate it. I suspect that in the process of doing this it's quite likely to fix the jumpy scale-bar issue, no promises.