Fix picks on clipped regions

[ggetz]

Description

This PR fixes a uniform error which can arise when picking 3D Tilesets that have a region clipped by ClippingPolygonCollection.

As @javagl helpfully pointed out, the ClippingPolygonCollection._extentsTexture property is set during the collection update (the webgl context, required to create the Texture, is not available until then). The error can manifest during picking, particularly clampToHeight , which attempts to render sooner than expected in the process. This leads to an uniform error.

Instead, this PR simply defaults to the default empty texture in the case where the texture has not yet had the chance to initialize.

Issue number and link

Fixes #12725

Testing plan

You will need to open the developer tools in order to replicate the bug.

• main, with error when you drag and move the polygon
• this branch, no errors when you drag and move the polygon

I did not add a unit test since this is an edge case that is difficult to construct a sensible test for.

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @ggetz!

✅ We can confirm we have a CLA on file for you.

[ggetz]

@mzschwartz5 Could you please review? It'd be great to get this fix into Tuesday's release for the ion team.

[javagl]

You have seen in the issue that I did go down the rabbit hole a bit, but didn't bother to look for the "Drink me" bottle.

Two small comments:

For me, the issue only happened when the debug console was open. One can shrug this off: "It's a browser, not a VM 🤷". But is there any convincing reason for that?

The change here prevents the crash. Let's hit "Merge" 😉
But ... I'd at least consider to add

      return (
        // When [something happens] then [in some cases] and [when the browser console is open(!?)]
        // this may be called by [some function], at a point in time where this texture is not yet initialized.
        // Using the default texture is a workaound for https://github.com/CesiumGS/cesium/issues/12725
        clippingPolygons.clippingTexture ?? frameState.context.defaultTexture
      );

And... it's indeed only a workaround to prevent the crash. Whatever this _extentsTexture is (I don't know that), it did probably contain specific values, and these values have been used at the consuming side, to perform some sort of computation. Now, whatever "value" that was, it is now the value for "white" for a short moment. And depending on what was done with the value, it's hard to predict the effect of this. (Some unpredictable area being clipped? Maybe showing up as soms sort of "flicker"? And.... maybe, one day, we change the color for the default texture to magenta, and suddenly, the clipping does no longer work....?).
(I'm not saying that this "patch" is the wrong way to solve it. I just want to make sure that it is the right way to solve it...)

[ggetz]

The change here prevents the crash. Let's hit "Merge" 😉

My comment might end up boiling down to this, but...

Thanks for for flagging the nuances @javagl — you’re right that this is more of a workaround than a root-cause fix, and your notes about the fallback texture are valid. That said, since this crash is currently blocking ion, I’d suggest we move forward with this patch as an immediate safeguard.

To your points:

1. I ran this and related clipping examples and didn’t observe any behavioral differences.
2. I suspect the “only when console is open” effect comes from devtools overhead exposing a race condition.
3. While the fallback isn’t a perfect replacement, in the cases where it triggers it should only be used for about a frame of a non-rendering pass — and using zero values should effectively result in “no clipping.”

So my recommendation is: let’s merge this as a stopgap, and if there are still concerns on the underlying issue, let's file a follow-up issue to better re-evaluate timing of texture initialization.

[mzschwartz5]

Taking a review pass now - just noticed right off the bat, though, that I'm unable to replicate this in Firefox. No problem in Chrome though. Don't know if that's worth anything but I hadn't seen it mentioned anywhere yet.

[mzschwartz5]

The issue description said part of the problem is the lack of an available WebGL context until the Update function in ClippingPolygonCollection. Clearly though, we have a valid context here, at least.

Question - rather than returning the defaultTexture, could we call a function on the clippingPolygons object here to create the clippingTexture if it's undefined at this point? (This function I'm referring to doesn't exist yet, but could we just refactor the code in Update that creates the texture into a callable function?)

If that seems like a big refactor / we need this sooner and it works as is, I'm fine shipping it.

[mzschwartz5]

(Regardless, I'll hold off merging any code changes until people are back in office)