Skip to content

Conversation

@yorkie
Copy link
Member

@yorkie yorkie commented Jul 31, 2025

This PR supports rendering web content in layers by scrollable containers, this is required to achieve the complete overflow implementation.

@yorkie yorkie requested a review from Copilot July 31, 2025 11:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for rendering web content in layers by introducing a new RenderLayer component and updating the rendering pipeline to support transparent materials. The changes enable proper rendering of scrollable containers with overflow handling by supporting both opaque and transparent render passes.

  • Introduces RenderLayer component to support layered rendering
  • Updates web content materials to use transparent rendering instead of opaque
  • Removes render pass filtering to allow both opaque and transparent passes

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
scene.hpp Adds include for new RenderLayer header
scene.cpp Registers RenderLayer component and adds it to entity creation
scene_renderer.cpp Adds stencil buffer clearing before volume mask rendering
mesh_material.hpp Adds transparency checks and render pass matching logic
web_content_instanced.cpp Changes web content material from opaque to transparent
material_base.hpp Adds constructor overload to specify opacity
client_renderer.hpp Removes documentation comments
client_renderer.cpp Removes render pass filtering and adds material pass matching

*/
inline bool matchesPass(const RenderPass &pass) const
{
assert(pass == RenderPass::kOpaques || pass == RenderPass::kTransparents);
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion duplicates the logic in the if-else conditions below. Consider removing the assertion or simplifying the function to avoid redundant checks.

Suggested change
assert(pass == RenderPass::kOpaques || pass == RenderPass::kTransparents);

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +82
else if (pass == RenderPass::kTransparents)
return isTransparent();
else
return false;
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final else clause will never execute due to the assertion above. If the assertion is removed, this should be unreachable code. Consider removing this clause or handling unexpected enum values appropriately.

Suggested change
else if (pass == RenderPass::kTransparents)
return isTransparent();
else
return false;
else // pass == RenderPass::kTransparents
return isTransparent();

Copilot uses AI. Check for mistakes.

WebContentInstancedMaterial::WebContentInstancedMaterial()
: Material()
: Material(false) // Web content must use transparent material
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment explains why web content uses transparent material, but it would be helpful to explain the relationship to overflow implementation or layer rendering mentioned in the PR description.

Suggested change
: Material(false) // Web content must use transparent material
: Material(false) // Web content must use transparent material to ensure proper rendering of layers and handling of overflow. Transparent material allows blending and compositing of web content layers, which is essential for implementing overflow behavior and maintaining visual fidelity.

Copilot uses AI. Check for mistakes.
if (!materialComponent->initialized())
renderer_->initializeMeshMaterial3d(meshComponent, materialComponent);

// Skip rendering if the material does not for this render pass, such as the mesh is not opaque but the render pass
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar error: 'does not for this render pass' should be 'does not match this render pass' or similar.

Suggested change
// Skip rendering if the material does not for this render pass, such as the mesh is not opaque but the render pass
// Skip rendering if the material does not match this render pass, such as when the mesh is not opaque but the render pass

Copilot uses AI. Check for mistakes.
@yorkie yorkie merged commit 9a6761f into main Jul 31, 2025
2 checks passed
@yorkie yorkie deleted the feat/render-in-layers branch July 31, 2025 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants