Trigger a rendering when a data source becomes ready

[javagl]

Description

This only triggers a requestRender() when a DataSource becomes 'ready' for the first time, in DataSourceDisplay#update.

This fixes #11928
(Which was a regression from #11855 )

Testing plan

Until now, I tested this with the Sandcastle from the original issue. The behavior now is that when the data source finishes loading (as indicated by the 'Loaded' console message), a requestRender is triggered and the geometry appears without moving the camera.

I also tested it with the "Case 1, Primitive" sandcastle from the PR #11855, just to check that this behaves as before.

For unit tests: I'll have to check how this can be tested sensibly. There probably already is a test along the lines of

createViewerWith( requestRenderMode: true );
addSomeDataSource(...)
scene.postRender.addEventListener(() => {
    shouldBeCalled();
}

But I first wanted to confirm that this is the right "layer" of addressing this issue. (It could probably be solved in a million different ways, and it's not unlikely that there is a "better" way to solve it...)

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 update 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, @javagl!

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

[ggetz]

Thanks @javagl! I do think this is an appropriate fix for the specific issue.

Could you please update CHANGES.md?

This may be a bit tricky to test the specific of this fix given that completed requests will also trigger renders. One option would be to start listening once the datasource request is complete.

[javagl]

I updated the CHANGES.md, and added a test, roughly following the pattern from similar specs. Should I try to cover some flow like

when (becomes ready during update) expect(renderRequested to be true);
when (remains ready during update) expect(renderRequested to be false);

?
(It looks like scene.renderForSpecs() does not reset the scene._renderRequested flag, so I might have to look closer into that...)

[ggetz]

Thanks @javagl! Any additional coverage would be appreciated.

Your point about the _renderRequested flag not being reset indicates that this feature probably isn't a finely covered in the unit tests.

[javagl]

Your point about the _renderRequested flag not being reset indicates that this feature probably isn't a finely covered in the unit tests.

Well, upon further investigation, it was reset to false, but then immediately set to true again, via a ... not-so-obvious mechanism that I described in a short inline comment. (If this falls into the category of "Don't pollute test code with information that everybody knows", I can remove it...)

[javagl]

@ggetz I don't see a connection between the changes in this PR and the current build failures. But if there could be a connection, I can have another look...

[ggetz]

@javagl I agree the CI failure is unrelated. I'll open a new issue to track.

[ggetz]

Thanks @javagl! This is looking good to me.