Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] set GLES viewport once, remove extra unbind/validation. #56902

Merged
auto-submit merged 1 commit into from
Dec 3, 2024
Merged

[Impeller] set GLES viewport once, remove extra unbind/validation. #56902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 39 additions & 25 deletions impeller/renderer/backend/gles/render_pass_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
const auto& gl = reactor.GetProcTable();
#ifdef IMPELLER_DEBUG
tracer->MarkFrameStart(gl);
#endif // IMPELLER_DEBUG

fml::ScopedCleanupClosure pop_pass_debug_marker(
Copy link
Contributor Author

@jonahwilliams jonahwilliams Dec 2, 2024

Choose a reason for hiding this comment

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

all of this should be ifdef'd out.

[&gl]() { gl.PopDebugGroup(); });
Expand All @@ -205,6 +204,7 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
} else {
pop_pass_debug_marker.Release();
}
#endif // IMPELLER_DEBUG

GLuint fbo = GL_NONE;
TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment);
Expand Down Expand Up @@ -285,6 +285,29 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {

gl.Clear(clear_bits);

// Both the viewport and scissor are specified in framebuffer coordinates.
// Impeller's framebuffer coordinate system is top left origin, but OpenGL's
// is bottom left origin, so we convert the coordinates here.
auto target_size = pass_data.color_attachment->GetSize();

//--------------------------------------------------------------------------
/// Setup the viewport.
///
const auto& viewport = pass_data.viewport;
gl.Viewport(viewport.rect.GetX(), // x
target_size.height - viewport.rect.GetY() -
viewport.rect.GetHeight(), // y
viewport.rect.GetWidth(), // width
viewport.rect.GetHeight() // height
);
if (pass_data.depth_attachment) {
if (gl.DepthRangef.IsAvailable()) {
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
} else {
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
}
}

for (const auto& command : commands) {
if (command.instance_count != 1u) {
VALIDATION_LOG << "GLES backend does not support instanced rendering.";
Expand Down Expand Up @@ -339,26 +362,24 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
gl.Disable(GL_DEPTH_TEST);
}

// Both the viewport and scissor are specified in framebuffer coordinates.
// Impeller's framebuffer coordinate system is top left origin, but OpenGL's
// is bottom left origin, so we convert the coordinates here.
auto target_size = pass_data.color_attachment->GetSize();

//--------------------------------------------------------------------------
/// Setup the viewport.
///
const auto& viewport = command.viewport.value_or(pass_data.viewport);
gl.Viewport(viewport.rect.GetX(), // x
target_size.height - viewport.rect.GetY() -
viewport.rect.GetHeight(), // y
viewport.rect.GetWidth(), // width
viewport.rect.GetHeight() // height
);
if (pass_data.depth_attachment) {
if (gl.DepthRangef.IsAvailable()) {
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
} else {
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
if (command.viewport.has_value()) {
gl.Viewport(viewport.rect.GetX(), // x
target_size.height - viewport.rect.GetY() -
viewport.rect.GetHeight(), // y
viewport.rect.GetWidth(), // width
viewport.rect.GetHeight() // height
);
if (pass_data.depth_attachment) {
if (gl.DepthRangef.IsAvailable()) {
gl.DepthRangef(viewport.depth_range.z_near,
viewport.depth_range.z_far);
} else {
gl.DepthRange(viewport.depth_range.z_near,
viewport.depth_range.z_far);
}
}
}

Expand Down Expand Up @@ -481,13 +502,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
if (!vertex_desc_gles->UnbindVertexAttributes(gl)) {
return false;
}

//--------------------------------------------------------------------------
/// Unbind the program pipeline.
///
if (!pipeline.UnbindProgram()) {
return false;
}
}

if (gl.DiscardFramebufferEXT.IsAvailable()) {
Expand Down
9 changes: 0 additions & 9 deletions impeller/renderer/render_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ bool RenderPass::AddCommand(Command&& command) {
return false;
}

if (command.scissor.has_value()) {
auto target_rect = IRect::MakeSize(render_target_.GetRenderTargetSize());
if (!target_rect.Contains(command.scissor.value())) {
VALIDATION_LOG << "Cannot apply a scissor that lies outside the bounds "
"of the render target.";
return false;
}
}

if (command.element_count == 0u || command.instance_count == 0u) {
// Essentially a no-op. Don't record the command but this is not necessary
// an error either.
Expand Down
Loading