Add FramebufferManager class

[ebogo1]

This PR adds the FramebufferManager class, which wraps Framebuffer resources (textures, depth stencil textures, renderbuffers, and the framebuffer itself) in a way that can be used by multiple classes that share similar code in constructing their own framebuffers. In wrapper-msaa, FramebufferManager also supports the new MultisampleFramebuffer class, which will eventually make it easier to add MSAA to framebuffers that need it. To make the changes easier to see, this also removes the primitiveColorFramebuffer from GlobeDepth.js.

I think this should be opened as a draft PR first, because there are a few things I'm still unsure about:

• Some framebuffers are constructed with multiple texture attachments, and FramebufferManager currently expects a single attachment for color and depth stencil textures/renderbuffers. It feels clunky to have a textureOptions argument to keep track of constructor options in FramebufferManager.update, maybe if this is needed then it could go in the constructor instead? (resolved by adding constructor options)
• Some framebuffers depend on attachments that belong to another framebuffer. I think it's simplest to avoid the FramebufferManager class for this case and to construct a Framebuffer manually instead. (resolved by allowing shared attachments to be added to a FramebufferManager manually)
• Should the goal be for every class to use the FramebufferManager constructor instead of directly creating Framebuffer objects? If not, should the FramebufferManager class be tailored to framebuffers that will need MSAA support?
• Which framebuffers need MSAA? Some candidates are the ones in PickDepth, PickDepthFramebuffer, ShadowMap, GlobeTranslucencyFramebuffer, OIT, PointCloudEyeDomeLighting, and TranslucentTileClassification.

The MSAA issue is here - #9900

---

To-do:

• JS docs
• Write unit tests

[cesium-concierge]

Thanks for the pull request @ebogo1!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.

[lilleyse]

SceneFramebuffer and PickDepth can't be trimmed down much further but there's still a lot of room for improvement in GlobeDepth. I'm mainly thinking about the render state and scissor code which is in four different files and somewhat complex and mistake prone. I don't have a clear idea of how this might be integrated in FramebufferManager, if at all, but keep it in mind as you update the rest of the code.

[lilleyse]

• Some framebuffers depend on attachments that belong to another framebuffer. One example is GlobeDepth._updateDepthFramebuffer - I think it's simplest to avoid the FramebufferManager class for this case and to construct a Framebuffer manually instead.

While unfortunate I agree. Generally we want a FramebufferManager to own its resources. If this comes up again in some other area of the code we could reconsider.

[lilleyse]

• Should the goal be for every class to use the FramebufferManager constructor instead of directly creating Framebuffer objects? If not, should the FramebufferManager class be tailored to framebuffers that will need MSAA support?

Yes, the FramebufferManager should be used wherever possible.

[lilleyse]

• Which framebuffers need MSAA? Some candidates are the ones in PickDepth, PickDepthFramebuffer, ShadowMap, GlobeTranslucencyFramebuffer, OIT, PointCloudEyeDomeLighting, and TranslucentTileClassification.

I don't have a concrete answer right now, but I think it will become clear as you start to update the other classes. General rule of thumb though is that any framebuffer that gets geometry rendered to it will need MSAA. This excludes framebuffers only used in full screen passes.

[ebogo1]

@lilleyse ShadowMap is an interesting case, since the class doesn't own any of the Framebuffers or attachments it creates. This could change, but then ShadowMap would store more resources than it does now which is kind of the opposite of what I've been using FramebufferManager for. Any thoughts?

cesium/Source/Scene/ShadowMap.js

Lines 621 to 629 in 554baa1

	function createFramebuffer(shadowMap, context) {
	if (shadowMap._isPointLight) {
	createFramebufferCube(shadowMap, context);
	} else if (shadowMap._usesDepthTexture) {
	createFramebufferDepth(shadowMap, context);
	} else {
	createFramebufferColor(shadowMap, context);
	}
	}

[ebogo1]

@lilleyse As of e42e8f8 this PR is almost ready to be undrafted.

Classes that are still constructing framebuffers manually (trimmed down after 7917da9) :

• ComputeEngine constructs a framebuffer for use in commands that are executed and then destroys the framebuffer right after. I'm not sure this is a good use case for FramebufferManager.
• ShadowMap constructs framebuffers with different sets of attachments based on variables that I believe can change at runtime (i.e. isPointLight). Also, between the cubemap color textures and regular color texture, I think the value of FramebufferManager._createColorAttachments would also need to be changed at runtime. It is doable to change this code to use FramebufferManager but I'm still not sure if it's a "good" use of the class unless the API is changed to give more runtime control over attachment management.
• TextureAtlas is another case of creating a framebuffer and immediately destroying (and unbinding) it just to copy a texture over. This could probably use FramebufferManager but after a first pass that doesn't seem like an improvement over the current code.

Still to-do:

• More docs where needed
• FramebufferManager specs
• Validate edge cases in browser

[ebogo1]

This behavior may cause errors to be avoided where previously they would be thrown. i.e., if a class used to always create depth textures without checking context.depthTexture, an error would be thrown when the extension was unsupported. Instead, this now creates a renderbuffer if supportsDepthTexture is disabled or if the extension is unsupported.

I noticed this in PickDepthFramebuffer and PointCloudEyeDomeLighting, but there might be a couple similar places.

[lilleyse]

I still have a little ways to go but here's everything up to InvertClassification

[lilleyse]

@ebogo1 that should cover the rest of the files

[lilleyse]

This is a pretty far reaching change so I went through a bunch of sandcastles to see if I could trigger any unexpected behavior. I also wanted to make sure that framebuffers were only recreated as needed, such as when the screen is resized or if HDR is toggled. I can't promise I've tested every corner case but things seem to be working well so far. Here's what I tested.

• Tested that AutoExposure works by setting this._autoExposureEnabled = true in PostProcessStageCollection and loading the High Dynamic Range sandcastle. It has the same behavior as main, which is good, but I don't think the exposure is actually working properly in either branch.
• BrdfLutGenerator - tested with the Image-Based Lighting sandcastle
• GlobeDepth - tested several things
  • Made sure czm_globeDepthTexture was working correctly. It's used by billboards that are clamped to ground as a coarse grained depth test (see BillboardClampToGround). It is also used by GlobeTranslucency when requiresManualDepthTest is true which can be triggered by sandcastle1 in Globe translucency #8726 when Front Translucent Back Opaque is selected and Depth test against terrain is unchecked. Also used by ground polylines and textured ground primitives such as in Classification Types. This last example also tests that executeUpdateDepth is working correctly. Picking the polygons and polylines works as well.
  • Checked that post processing effects that use depth textures work: Depth of Field, Fog Post Process
• Tested invert classification: Classification
• Tested viewsheds with invert classification in cesium-analytics: Sandcastle
• OIT has three different modes based on which WebGL extensions are supported. I tested these by modifying Context.js.
  • colorBufferFloat: true, depthTexture: true, drawBuffers: false
  • colorBufferFloat: true, depthTexture: true, drawBuffers: true
  • colorBufferFloat: false, depthTexture: false, drawBuffers: false
• PickDepth - checked that scene.pickPosition works, e.g. Drawing on Terrain with log depth enabled and disabled
• PickDepthFramebuffer - checked that scene.pickTranslucentDepth and scene.pickPosition work - cherry-picked the code from Fixed rendering issues related to environmentState.renderTranslucentDepthForPick. #9844 and ran the sandcastle from here
• PickFramebuffer - checked that scene.pick works
• EyeDomeLighting - checked Montreal Point Cloud. Previously @ebogo1 ran through the linked issues in Fix eye dome lighting crashes #9719
• PostProcessStageTextureCache - checked most of the post processing sandcastles
• SceneFramebuffer - spot checked a few sandcastle examples with context.depthTexture returning false
• SunPostProcess - angled the camera so the sun intersects the horizon
• TranslucentTileClassification - tested the sandcastle from here

@ebogo1 as a final test could you go through all the sandcastles as if you were doing a release? I suggest doing something similar to what I did - print a message whenever a new framebuffer is created in FramebufferBuffer and make sure it gets printed only when it makes sense like when the screen is resized or HDR changes. Also check if there's any behavior I didn't test for.

[ebogo1]

Going through all the Sandcastles, nothing looks out of place. Most framebuffers get created at runtime and when the canvas size changes. A couple other things I checked:

• No duplicate framebuffer or resource creation as far as I can tell; tested by checking when the body of FramebufferManager.update gets reached
• InvertClassification with translucency creates the same framebuffers as main when translucency is enabled/disabled or changed
• OIT and GlobeDepth recreate their framebuffers when HDR is toggled on/off - HDR Sandcastle
• Tested the 3D Tiles Next Photogrammetry Classification Sandcastle to make sure picking works, especially with the translucent windows option when a different pickable surface is visible behind a pickable window (also made sure this worked after resizing the canvas, just in case)
• Confirmed that the same number of framebuffers get created on this branch vs main in the Multiple Synced Views Sandcastle, which is a special case that creates the PickFramebuffer multiple times

[lilleyse]

@ebogo1 cool, should be good to merge after the January 3rd release.

[ebogo1]

@lilleyse I added a CHANGES.md entry for the 1.90 release - if the entry looks good this should be good to merge once CI passes.

edit - do we want an entry for these changes? It's a big change but most of the new API is in a private class..

[lilleyse]

edit - do we want an entry for these changes? It's a big change but most of the new API is in a private class..

No need to add to CHANGES.md since it's all private and invisible to the user. I'll remove the commit and then merge.