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

Commit 349ad27

Browse files
author
Jonah Williams
authored
[Impeller] set GLES viewport once, remove extra unbind/validation. (#56902)
Small cleanups to render_pass_gles to only set the viewport once or when it cahnges, instead of each draw. Removes scissor validation and pipeline unbind (which is slow due to hash table lookup).
1 parent c9fa56d commit 349ad27

2 files changed

Lines changed: 39 additions & 34 deletions

File tree

impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
196196
const auto& gl = reactor.GetProcTable();
197197
#ifdef IMPELLER_DEBUG
198198
tracer->MarkFrameStart(gl);
199-
#endif // IMPELLER_DEBUG
200199

201200
fml::ScopedCleanupClosure pop_pass_debug_marker(
202201
[&gl]() { gl.PopDebugGroup(); });
@@ -205,6 +204,7 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
205204
} else {
206205
pop_pass_debug_marker.Release();
207206
}
207+
#endif // IMPELLER_DEBUG
208208

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

286286
gl.Clear(clear_bits);
287287

288+
// Both the viewport and scissor are specified in framebuffer coordinates.
289+
// Impeller's framebuffer coordinate system is top left origin, but OpenGL's
290+
// is bottom left origin, so we convert the coordinates here.
291+
auto target_size = pass_data.color_attachment->GetSize();
292+
293+
//--------------------------------------------------------------------------
294+
/// Setup the viewport.
295+
///
296+
const auto& viewport = pass_data.viewport;
297+
gl.Viewport(viewport.rect.GetX(), // x
298+
target_size.height - viewport.rect.GetY() -
299+
viewport.rect.GetHeight(), // y
300+
viewport.rect.GetWidth(), // width
301+
viewport.rect.GetHeight() // height
302+
);
303+
if (pass_data.depth_attachment) {
304+
if (gl.DepthRangef.IsAvailable()) {
305+
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
306+
} else {
307+
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
308+
}
309+
}
310+
288311
for (const auto& command : commands) {
289312
if (command.instance_count != 1u) {
290313
VALIDATION_LOG << "GLES backend does not support instanced rendering.";
@@ -339,26 +362,24 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
339362
gl.Disable(GL_DEPTH_TEST);
340363
}
341364

342-
// Both the viewport and scissor are specified in framebuffer coordinates.
343-
// Impeller's framebuffer coordinate system is top left origin, but OpenGL's
344-
// is bottom left origin, so we convert the coordinates here.
345-
auto target_size = pass_data.color_attachment->GetSize();
346-
347365
//--------------------------------------------------------------------------
348366
/// Setup the viewport.
349367
///
350-
const auto& viewport = command.viewport.value_or(pass_data.viewport);
351-
gl.Viewport(viewport.rect.GetX(), // x
352-
target_size.height - viewport.rect.GetY() -
353-
viewport.rect.GetHeight(), // y
354-
viewport.rect.GetWidth(), // width
355-
viewport.rect.GetHeight() // height
356-
);
357-
if (pass_data.depth_attachment) {
358-
if (gl.DepthRangef.IsAvailable()) {
359-
gl.DepthRangef(viewport.depth_range.z_near, viewport.depth_range.z_far);
360-
} else {
361-
gl.DepthRange(viewport.depth_range.z_near, viewport.depth_range.z_far);
368+
if (command.viewport.has_value()) {
369+
gl.Viewport(viewport.rect.GetX(), // x
370+
target_size.height - viewport.rect.GetY() -
371+
viewport.rect.GetHeight(), // y
372+
viewport.rect.GetWidth(), // width
373+
viewport.rect.GetHeight() // height
374+
);
375+
if (pass_data.depth_attachment) {
376+
if (gl.DepthRangef.IsAvailable()) {
377+
gl.DepthRangef(viewport.depth_range.z_near,
378+
viewport.depth_range.z_far);
379+
} else {
380+
gl.DepthRange(viewport.depth_range.z_near,
381+
viewport.depth_range.z_far);
382+
}
362383
}
363384
}
364385

@@ -481,13 +502,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
481502
if (!vertex_desc_gles->UnbindVertexAttributes(gl)) {
482503
return false;
483504
}
484-
485-
//--------------------------------------------------------------------------
486-
/// Unbind the program pipeline.
487-
///
488-
if (!pipeline.UnbindProgram()) {
489-
return false;
490-
}
491505
}
492506

493507
if (gl.DiscardFramebufferEXT.IsAvailable()) {

impeller/renderer/render_pass.cc

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,6 @@ bool RenderPass::AddCommand(Command&& command) {
6363
return false;
6464
}
6565

66-
if (command.scissor.has_value()) {
67-
auto target_rect = IRect::MakeSize(render_target_.GetRenderTargetSize());
68-
if (!target_rect.Contains(command.scissor.value())) {
69-
VALIDATION_LOG << "Cannot apply a scissor that lies outside the bounds "
70-
"of the render target.";
71-
return false;
72-
}
73-
}
74-
7566
if (command.element_count == 0u || command.instance_count == 0u) {
7667
// Essentially a no-op. Don't record the command but this is not necessary
7768
// an error either.

0 commit comments

Comments
 (0)