[Impeller] Enable MSAA for OpenGLES: Take 2. #47030

matanlurey · 2023-10-17T21:59:32Z

Continues the work started in #46381. This is PR supercedes #46688.

It's worth calling out the mechanism we're using is only supported in OpenGL 3.0+, so we'll need a different solution (either by default, or when Blit is not available) to get proper device support. I'll file an issue before merging.

Status

Appears to work! I validated it on a local demo app, flutter_gallery, and Wonderous.

Example:

Background

History

Still blocked, but MSAA is working provided you use a phone with OpenGLES 3.0+. The last bit is getting stencil attachments to work again (they currently crash with a GL_INVALID_OPERATION).

Compared to #46688, we''ve corrected some incorrect OpenGL calls and assumptions - for example we now have both multi-sampled textures similar to the MultisampledFBO.cpp example.

After doing so, the GL driver is successfully called, and no errors or crashes persist. Yay!

We did need to use glBlitFramebuffer to move from the resolve texture to the color attachment, and referenced some other example code for that.

Example App

import 'package:flutter/material.dart';

void main() {
  runApp(const MainApp());
}

class MainApp extends StatefulWidget {
  const MainApp({super.key});

  @override
  State<MainApp> createState() => _MainAppState();
}

class _MainAppState extends State<MainApp> {
  bool msaa = true;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          title: const Text('Force offscreen MSAA'),
          actions: [
            Switch(
              value: msaa,
              onChanged: (value) {
                setState(() {
                  msaa = value;
                });
              },
            ),
          ],
        ),
        body: Center(
          child: ForceOffscreenMSAA(opaque: !msaa),
        ),
      ),
    );
  }
}

// Draws 2 overlapping circles (BoxDecoration/BoxShape) wrapped in 50% opacity.
class ForceOffscreenMSAA extends StatelessWidget {
  final bool opaque;

  const ForceOffscreenMSAA({required this.opaque, super.key});

  @override
  Widget build(BuildContext context) {
    return Opacity(
      opacity: opaque ? 1.0 : 0.5,
      child: const DecoratedBox(
        decoration: BoxDecoration(
          shape: BoxShape.circle,
          color: Colors.red,
        ),
        child: SizedBox(
          width: 300,
          height: 300,
          child: DecoratedBox(
            decoration: BoxDecoration(
              shape: BoxShape.circle,
              color: Colors.green,
            ),
          ),
        ),
      ),
    );
  }
}

Screenshots

Open GL Commands during MSAA Render

glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 1367, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x6f9900a578)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 4480, , 35044)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x0)
glUniformMatrix4fv(, 0, 1, 0, 0xb4000070aadbc860)
glUniform4fv(, 1, 1, 0xb4000070aadbc8a0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0x480)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1369, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x880)
glUniformMatrix4fv(, 0, 1, 0, 0xb4000070aadbd160)
glUniform4fv(, 1, 1, 0xb4000070aadbd1a0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0xd00)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glDiscardFramebufferEXT(, 36160, 3, 0xb4000071cad5b590)
glBindFramebuffer(, 36160, 0)
glDeleteFramebuffers(, 1, 0x6f9900b25c)
glPopDebugGroupKHR()
glDeleteBuffers(, 1, 0x6f9900ce98)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 1370, 39, EntityPass Render Pass: Depth=0 Count=0)
glClearColor(, 1, 0.984314, 0.996078, 1)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 1371, 21, Texture Fill: Subpass)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x6f9900bfc8)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 11360, , 35044)
glUseProgram(, 57)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 16, 0x0)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 2, 5126, 0, 16, 0x8)
glUniformMatrix4fv(, 1, 1, 0, 0xb40000705ad2f200)
glUniform1fv(, 2, 1, 0xb40000705ad2f240)
glUniform1fv(, 3, 1, 0xb40000705ad2f244)
glActiveTexture(, 33984)
glBindTexture(, 3553, 1)
glTexParameteri(, 3553, 10241, 9728)
glTexParameteri(, 3553, 10240, 9728)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 1)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1373, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 58)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x180)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad2f300)
glUniform4fv(, 1, 1, 0xb40000705ad2f340)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1374, 14, Intersect Clip)
glEnable(, 3042)
glBlendFuncSeparate(, 0, 1, 0, 1)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, 
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7682)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 59)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x280)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad2f400)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1375, 9, TextFrame)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 1, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 60)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 4, 5126, 0, 48, 0x4c0)
glEnableVertexAttribArray(, 3)
glVertexAttribPointer(, 3, 2, 5126, 0, 48, 0x4e8)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 4, 5126, 0, 48, 0x4d0)
glEnableVertexAttribArray(, 2)
glVertexAttribPointer(, 2, 2, 5126, 0, 48, 0x4e0)
glUniformMatrix4fv(, 1, 1, 0, 0xb40000705ad2f500)
glUniformMatrix4fv(, 2, 1, 0, 0xb40000705ad2f540)
glUniform2fv(, 3, 1, 0xb40000705ad2f580)
glUniform2fv(, 4, 1, 0xb40000705ad2f588)
glUniform4fv(, 5, 1, 0xb40000705ad2f590)
glUniform1fv(, 6, 1, 0xb40000705ad2f5a0)
glActiveTexture(, 33984)
glBindTexture(, 3553, 2)
glTexParameteri(, 3553, 10241, 9728)
glTexParameteri(, 3553, 10240, 9728)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 4, 0, 120)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 3)
glDisableVertexAttribArray(, 1)
glDisableVertexAttribArray(, 2)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1376, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 1, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 61)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x1b40)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad30f00)
glUniform4fv(, 1, 1, 0xb40000705ad30f40)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 144, 5123, 0x1cd0)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1377, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 1, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 61)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x1e80)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad31200)
glUniform4fv(, 1, 1, 0xb40000705ad31240)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 114, 5123, 0x1fc0)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1378, 12, Restore Clip)
glEnable(, 3042)
glBlendFuncSeparate(, 0, 1, 0, 1)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, 
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7681)
glStencilFuncSeparate(, 1032, 513, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 62)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x2180)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad31300)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1379, 12, RRect Shadow)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 63)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x2240)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad31400)
glUniform4fv(, 1, 1, 0xb40000705ad31500)
glUniform2fv(, 2, 1, 0xb40000705ad31510)
glUniform1fv(, 3, 1, 0xb40000705ad31518)
glUniform1fv(, 4, 1, 0xb40000705ad3151c)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1380, 10, Solid Fill)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 64)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x2420)
glUniformMatrix4fv(, 0, 1, 0, 0xb40000705ad31600)
glUniform4fv(, 1, 1, 0xb40000705ad31640)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 1381, 9, TextFrame)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 60)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 4, 5126, 0, 48, 0x26c0)
glEnableVertexAttribArray(, 3)
glVertexAttribPointer(, 3, 2, 5126, 0, 48, 0x26e8)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 4, 5126, 0, 48, 0x26d0)
glEnableVertexAttribArray(, 2)
glVertexAttribPointer(, 2, 2, 5126, 0, 48, 0x26e0)
glUniformMatrix4fv(, 1, 1, 0, 0xb40000705ad31700)
glUniformMatrix4fv(, 2, 1, 0, 0xb40000705ad31740)
glUniform2fv(, 3, 1, 0xb40000705ad31780)
glUniform2fv(, 4, 1, 0xb40000705ad31788)
glUniform4fv(, 5, 1, 0xb40000705ad31790)
glUniform1fv(, 6, 1, 0xb40000705ad317a0)
glActiveTexture(, 33984)
glBindTexture(, 3553, 2)
glTexParameteri(, 3553, 10241, 9729)
glTexParameteri(, 3553, 10240, 9729)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 4, 0, 30)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 3)
glDisableVertexAttribArray(, 1)
glDisableVertexAttribArray(, 2)
glUseProgram(, 0)
glPopDebugGroupKHR()
glDiscardFramebufferEXT(, 36160, 2, 0xb4000071cad5b310)
glPopDebugGroupKHR()

AGI Trace

~~As you can see, with MSAA enabled I get a weird mostly-blank artifact (or sometimes entirely blank).~~ UPDATE: FIXED.

What we tried (h/t @jonahwilliams):

Using AGI (with the ANGLE adapter, see also these oddities), see CircleOpacityTrace.gfxtrace.zip.
Print debugging the commands (see above)
Commenting out gl.DiscardFramebufferEXT optimizations
Render Doc (I need to use a Samsung phone old enough to have OpenGLES)

/cc @jonahwilliams to add anything else ^

matanlurey · 2023-10-18T20:10:56Z

@jonahwilliams and I pulled this up on a Samsung (native OpenGLES) phone and RenderDoc:

It appears the offscreen frame-buffer is not actually painting the circle (a mesh exists).

Simplifying the demo (removing everything but the circle) results in the following OpenGL commands:

glGenRenderbuffers(, 1, 0x7a5f04bca8)
glGenRenderbuffers(, 1, 0x7a5f04bca8)
glDeleteRenderbuffers(, 1, 0x7a5f048448)
glDeleteRenderbuffers(, 1, 0x7a5f048448)
glDeleteBuffers(, 1, 0x7a5f048448)
glIsRenderbuffer(, 4)
glIsRenderbuffer(, 3)
glObjectLabelKHR(, 36161, 3, 38, EntityPass Color Texture (Multisample))
glIsTexture(, 1)
glObjectLabelKHR(, 5890, 1, 24, EntityPass Color Texture)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 118, 39, EntityPass Render Pass: Depth=1 Count=0)
glGenFramebuffers(, 1, 0x7a5f04825c)
glBindFramebuffer(, 36160, 1)
glFramebufferRenderbuffer(, 36160, 36064, 36161, 3)
glBindRenderbuffer(, 36161, 0)
glCheckFramebufferStatus(, 36160)
glClearColor(, 0, 0, 0, 0)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 119, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a5f0475d8)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 4480, , 35044)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x0)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0b5020)
glUniform4fv(, 1, 1, 0xb400007c3d0b5060)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0x480)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 121, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x880)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0b5920)
glUniform4fv(, 1, 1, 0xb400007c3d0b5960)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0xd00)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glBindFramebuffer(, 36160, 0)
glDeleteFramebuffers(, 1, 0x7a5f04825c)
glPopDebugGroupKHR()
glIsRenderbuffer(, 4)
glDeleteBuffers(, 1, 0x7a5f049e98)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 122, 39, EntityPass Render Pass: Depth=0 Count=0)
glClearColor(, 1, 0.984314, 0.996078, 1)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 123, 21, Texture Fill: Subpass)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a5f049028)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 2912, , 35044)
glUseProgram(, 57)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 16, 0x0)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 2, 5126, 0, 16, 0x8)
glUniformMatrix4fv(, 1, 1, 0, 0xb400007b6db102e0)
glUniform1fv(, 2, 1, 0xb400007b6db10320)
glUniform1fv(, 3, 1, 0xb400007b6db10324)
glActiveTexture(, 33984)
glBindTexture(, 3553, 1)
glTexParameteri(, 3553, 10241, 9728)
glTexParameteri(, 3553, 10240, 9728)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 1)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 125, 12, RRect Shadow)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 58)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x180)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007b6db103e0)
glUniform4fv(, 1, 1, 0xb400007b6db104e0)
glUniform2fv(, 2, 1, 0xb400007b6db104f0)
glUniform1fv(, 3, 1, 0xb400007b6db104f8)
glUniform1fv(, 4, 1, 0xb400007b6db104fc)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 126, 10, Solid Fill)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 59)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x320)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007b6db105e0)
glUniform4fv(, 1, 1, 0xb400007b6db10620)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 127, 9, TextFrame)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 60)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 4, 5126, 0, 48, 0x5c0)
glEnableVertexAttribArray(, 3)
glVertexAttribPointer(, 3, 2, 5126, 0, 48, 0x5e8)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 4, 5126, 0, 48, 0x5d0)
glEnableVertexAttribArray(, 2)
glVertexAttribPointer(, 2, 2, 5126, 0, 48, 0x5e0)
glUniformMatrix4fv(, 1, 1, 0, 0xb400007b6db106e0)
glUniformMatrix4fv(, 2, 1, 0, 0xb400007b6db10720)
glUniform2fv(, 3, 1, 0xb400007b6db10760)
glUniform2fv(, 4, 1, 0xb400007b6db10768)
glUniform4fv(, 5, 1, 0xb400007b6db10770)
glUniform1fv(, 6, 1, 0xb400007b6db10780)
glActiveTexture(, 33984)
glBindTexture(, 3553, 2)
glTexParameteri(, 3553, 10241, 9729)
glTexParameteri(, 3553, 10240, 9729)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 4, 0, 30)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 3)
glDisableVertexAttribArray(, 1)
glDisableVertexAttribArray(, 2)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPopDebugGroupKHR()

matanlurey · 2023-10-18T21:32:58Z

Here is a reduced RenderDoc capture with the debug banner hidden as well:

 EID  | Event                                                                                               | Action #
----------------------------------------------------------------------------------------------------------------------
2     | - EntityPass Render Pass: Depth=1 Count=0                                                           | 1-4   
13    |  \- glClear(Color = <0.000000, 0.000000, 0.000000, 0.000000>, Depth = <1.000000>, Stencil = <0x00>) | 1     
14    |   - Solid Fill                                                                                      | 2-3   
34    |    \- glDrawElements(426)                                                                           | 2     
38    |   - Solid Fill                                                                                      | 3-4   
57    |    \- glDrawElements(426)                                                                           | 3     
63    | - EntityPass Render Pass: Depth=0 Count=0                                                           | 4-6   
72    |  \- glClear(Color = <1.000000, 0.984314, 0.996078, 1.000000>, Depth = <1.000000>, Stencil = <0x00>) | 4     
73    |   - Texture Fill: Subpass                                                                           | 5-6   
104   |    \- glDrawArrays(4)                                                                               | 5     
110   | - eglSwapBuffers(Backbuffer Color)                                                                  | 6

matanlurey · 2023-10-18T22:02:40Z

Ok, I see some weirdness in the GL calls.

MSAA Enabled

Emphasis mine.

glGenRenderbuffers(, 1, 0x7a6001dca8)
glGenRenderbuffers(, 1, 0x7a6001dca8)
glDeleteRenderbuffers(, 1, 0x7a6001a448)
glDeleteRenderbuffers(, 1, 0x7a6001a448)
glIsRenderbuffer(, 4)
glIsRenderbuffer(, 3)
glObjectLabelKHR(, 36161, 3, 38, EntityPass Color Texture (Multisample))
glIsTexture(, 1)
glObjectLabelKHR(, 5890, 1, 24, EntityPass Color Texture)
glDeleteBuffers(, 1, 0x7a6001a448)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 84, 39, EntityPass Render Pass: Depth=1 Count=0)
glGenFramebuffers(, 1, 0x7a6001a25c)
glBindFramebuffer(, 36160, 1)
glFramebufferRenderbuffer(, 36160, 36064, 36161, 3)
glBindRenderbuffer(, 36161, 0)
glCheckFramebufferStatus(, 36160)
glClearColor(, 0, 0, 0, 0)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 85, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a600195d8)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 4480, , 35044)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x0)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0a3fa0)
glUniform4fv(, 1, 1, 0xb400007c3d0a3fe0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0x480)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 87, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x880)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0a48a0)
glUniform4fv(, 1, 1, 0xb400007c3d0a48e0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0xd00)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glBindFramebuffer(, 36160, 0)
glDeleteFramebuffers(, 1, 0x7a6001a25c)
glPopDebugGroupKHR()
glIsRenderbuffer(, 4)
glDeleteBuffers(, 1, 0x7a6001be98)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 88, 39, EntityPass Render Pass: Depth=0 Count=0)
glClearColor(, 1, 0.984314, 0.996078, 1)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 89, 21, Texture Fill: Subpass)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a6001b028)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 384, , 35044)
glUseProgram(, 57)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 16, 0x0)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 2, 5126, 0, 16, 0x8)
glUniformMatrix4fv(, 1, 1, 0, 0xb400007b6db52700)
glUniform1fv(, 2, 1, 0xb400007b6db52740)
glUniform1fv(, 3, 1, 0xb400007b6db52744)
glActiveTexture(, 33984)
glBindTexture(, 3553, 1)
glTexParameteri(, 3553, 10241, 9728)
glTexParameteri(, 3553, 10240, 9728)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 1)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPopDebugGroupKHR()

MSAA Disabled

glGenRenderbuffers(, 1, 0x7a621d2ca8)
glGenRenderbuffers(, 1, 0x7a621d2ca8)
glDeleteRenderbuffers(, 1, 0x7a621cf448)
glDeleteRenderbuffers(, 1, 0x7a621cf448)
glDeleteBuffers(, 1, 0x7a621cf448)
glIsTexture(, 1)
glObjectLabelKHR(, 5890, 1, 24, EntityPass Color Texture)
glIsRenderbuffer(, 3)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 92, 39, EntityPass Render Pass: Depth=1 Count=0)
glGenFramebuffers(, 1, 0x7a621cf25c)
glBindFramebuffer(, 36160, 1)
glFramebufferTexture2D(, 36160, 36064, 3553, 1, 0)
glCheckFramebufferStatus(, 36160)
glClearColor(, 0, 0, 0, 0)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 93, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a621ce5d8)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 4480, , 35044)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x0)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0a61b0)
glUniform4fv(, 1, 1, 0xb400007c3d0a61f0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0x480)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPushDebugGroupKHR(, 33354, 95, 10, Solid Fill)
glDisable(, 3042)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 787, 787)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glBindBuffer(, 34962, 1)
glUseProgram(, 56)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 8, 0x880)
glUniformMatrix4fv(, 0, 1, 0, 0xb400007c3d0a6ab0)
glUniform4fv(, 1, 1, 0xb400007c3d0a6af0)
glBindBuffer(, 34963, 1)
glDrawElements(, 4, 426, 5123, 0xd00)
glDisableVertexAttribArray(, 0)
glUseProgram(, 0)
glPopDebugGroupKHR()
glBindFramebuffer(, 36160, 0)
glDeleteFramebuffers(, 1, 0x7a621cf25c)
glPopDebugGroupKHR()
glDeleteBuffers(, 1, 0x7a621d0e98)
glIsRenderbuffer(, 3)
glDebugMessageControlKHR(, 4352, 4352, 4352, 0, nullptr, 1)
glPushDebugGroupKHR(, 33354, 96, 39, EntityPass Render Pass: Depth=0 Count=0)
glClearColor(, 1, 0.984314, 0.996078, 1)
glClearStencil(, 0)
glDisable(, 3089)
glDisable(, 2929)
glDisable(, 2960)
glDisable(, 2884)
glDisable(, 3042)
glColorMask(, 1, 1, 1, 1)
glClear(, 17664)
glPushDebugGroupKHR(, 33354, 97, 21, Texture Fill: Subpass)
glEnable(, 3042)
glBlendFuncSeparate(, 1, 771, 1, 771)
glBlendEquationSeparate(, 32774, 32774)
glColorMask(, , , , )
glEnable(, 2960)
glStencilOpSeparate(, 1032, 7680, 7680, 7680)
glStencilFuncSeparate(, 1032, 514, 0, 4294967295)
glStencilMaskSeparate(, 1032, 4294967295)
glDisable(, 2929)
glViewport(, 0, 0, 1080, 2029)
glDisable(, 3089)
glDisable(, 2884)
glFrontFace(, 2304)
glGenBuffers(, 1, 0x7a621d0028)
glBindBuffer(, 34962, 1)
glBufferData(, 34962, 384, , 35044)
glUseProgram(, 57)
glEnableVertexAttribArray(, 0)
glVertexAttribPointer(, 0, 2, 5126, 0, 16, 0x0)
glEnableVertexAttribArray(, 1)
glVertexAttribPointer(, 1, 2, 5126, 0, 16, 0x8)
glUniformMatrix4fv(, 1, 1, 0, 0xb400007b6db47650)
glUniform1fv(, 2, 1, 0xb400007b6db47690)
glUniform1fv(, 3, 1, 0xb400007b6db47694)
glActiveTexture(, 33984)
glBindTexture(, 3553, 1)
glTexParameteri(, 3553, 10241, 9728)
glTexParameteri(, 3553, 10240, 9728)
glTexParameteri(, 3553, 10242, 33071)
glTexParameteri(, 3553, 10243, 33071)
glUniform1i(, 0, 0)
glDrawArrays(, 5, 0, 4)
glDisableVertexAttribArray(, 0)
glDisableVertexAttribArray(, 1)
glUseProgram(, 0)
glPopDebugGroupKHR()
glPopDebugGroupKHR()

There is no call to gl.FramebufferTexture2D (or really, gl.FramebufferTexture2DMultisampleEXT) in the MSAA.

…s-msaa-take2

impeller/renderer/backend/gles/capabilities_gles.cc

matanlurey · 2023-10-23T19:55:14Z

Picking this back up and looking at the stencil failures.

With MSAA enabled, the following crashes:

// render_pass_gles.cc
if (!stencil->SetAsFramebufferAttachment(
        GL_FRAMEBUFFER, TextureGLES::AttachmentPoint::kStencil)) {
  return false;
}

// texture_gles.cc -> TextureGLES::SetAsFramebufferAttachment
gl.FramebufferTexture2DMultisampleEXT(
    target,                    // target
    ToAttachmentPoint(point),  // attachment
    GL_TEXTURE_2D,             // textarget
    handle.value(),            // texture
    0,                         // level
    4                          // samples
);

The stencil looks like the following:

StorageMode=DeviceTransient,
Type=Texture2DMultisample,
Format=S8UInt,
Size=(825, 824),
MipCount=1,
SampleCount=4,
Compression=Lossless

jonahwilliams · 2023-10-23T19:59:12Z

dumb idea, try changing the stencil configuration so that it looks like a non-multisampled texture even when MSAA is enabled.

matanlurey · 2023-10-23T20:00:33Z

It turns out the crash is just because the texture image is never setup:

Break on 'ImpellerValidationBreak' to inspect point of failure: Invalid format for texture image.

explicit TexImage2DData(PixelFormat pixel_format) {
  switch (pixel_format) {
    case PixelFormat::kA8UNormInt:
      internal_format = GL_ALPHA;
      external_format = GL_ALPHA;
      type = GL_UNSIGNED_BYTE;
      break;
    case PixelFormat::kR8G8B8A8UNormInt:
    case PixelFormat::kB8G8R8A8UNormInt:
    case PixelFormat::kR8G8B8A8UNormIntSRGB:
    case PixelFormat::kB8G8R8A8UNormIntSRGB:
      internal_format = GL_RGBA;
      external_format = GL_RGBA;
      type = GL_UNSIGNED_BYTE;
      break;
    case PixelFormat::kR32G32B32A32Float:
      internal_format = GL_RGBA;
      external_format = GL_RGBA;
      type = GL_FLOAT;
      break;
    case PixelFormat::kR16G16B16A16Float:
      internal_format = GL_RGBA;
      external_format = GL_RGBA;
      type = GL_HALF_FLOAT;
      break;
    case PixelFormat::kUnknown:
    case PixelFormat::kS8UInt:
    case PixelFormat::kD24UnormS8Uint:
    case PixelFormat::kD32FloatS8UInt:
    case PixelFormat::kR8UNormInt:
    case PixelFormat::kR8G8UNormInt:
    case PixelFormat::kB10G10R10XRSRGB:
    case PixelFormat::kB10G10R10XR:
    case PixelFormat::kB10G10R10A10XR:
      return;
  }
  is_valid_ = true;
}

jonahwilliams · 2023-10-23T20:08:57Z

Right, we hit this before. I was confused how this was working previously, because we will always create a stencil buffer for clips

flutter-dashboard · 2023-10-23T21:09:59Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

jonahwilliams · 2023-10-23T21:11:04Z

impeller/renderer/backend/gles/render_pass_gles.cc

+  //
+  // Do not use the resolve_texture, and instead use the MSAA texture only.
+  // To preview what this would look like (but should not be implemented this
+  // way - would impact the HAL):
+  //
+  //    @@ render_target.cc @@ CreateOffscreenMSAA
+  //    - color0.resolve_texture = color0_resolve_tex;
+  //    + color0.resolve_texture = color0_msaa_tex;


nit: I would remove this part.

This is mostly a reminder to myself 😅 but I can put it in the issue I end up filing instead:

// Do not use the resolve_texture, and instead use the MSAA texture only. // To preview what this would look like (but should not be implemented this // way - would impact the HAL): // // @@ render_target.cc @@ CreateOffscreenMSAA // - color0.resolve_texture = color0_resolve_tex; // + color0.resolve_texture = color0_msaa_tex;

jonahwilliams · 2023-10-23T21:11:41Z

impeller/renderer/backend/gles/texture_gles.cc

@@ -21,19 +22,20 @@ static TextureGLES::Type GetTextureTypeFromDescriptor(
  const auto usage = static_cast<TextureUsageMask>(desc.usage);
  const auto render_target =
      static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
-  if (usage == render_target) {
+  const auto is_msaa = desc.sample_count == SampleCount::kCount4;
+  if (usage == render_target && !is_msaa) {
    return TextureGLES::Type::kRenderBuffer;


Maybe file a bug to investigate if we should investigate RenderBuffer multisampled (now that we have something working)

jonahwilliams · 2023-10-23T21:12:31Z

impeller/renderer/backend/gles/texture_gles.cc

      case PixelFormat::kS8UInt:
+        internal_format = GL_DEPTH_STENCIL;


I think this is a combined depth stencil format, like kD24UnormS8Uint below. This is fine to use now but we should investigate if there is a stencil-only format.

Sg, I'll add it the MSAA next steps issue and added a TODO here.

jonahwilliams

LGTM with nits

matanlurey

Added 3 TODOs.

matanlurey · 2023-10-23T21:13:02Z

impeller/renderer/backend/gles/render_pass_gles.cc

+  //
+  // Do not use the resolve_texture, and instead use the MSAA texture only.
+  // To preview what this would look like (but should not be implemented this
+  // way - would impact the HAL):
+  //
+  //    @@ render_target.cc @@ CreateOffscreenMSAA
+  //    - color0.resolve_texture = color0_resolve_tex;
+  //    + color0.resolve_texture = color0_msaa_tex;


This is mostly a reminder to myself 😅 but I can put it in the issue I end up filing instead:

// Do not use the resolve_texture, and instead use the MSAA texture only. // To preview what this would look like (but should not be implemented this // way - would impact the HAL): // // @@ render_target.cc @@ CreateOffscreenMSAA // - color0.resolve_texture = color0_resolve_tex; // + color0.resolve_texture = color0_msaa_tex;

matanlurey · 2023-10-23T21:13:38Z

impeller/renderer/backend/gles/texture_gles.cc

      case PixelFormat::kS8UInt:
+        internal_format = GL_DEPTH_STENCIL;


Sg, I'll add it the MSAA next steps issue and added a TODO here.

matanlurey · 2023-10-23T21:20:36Z

impeller/renderer/backend/gles/texture_gles.cc

@@ -21,19 +22,20 @@ static TextureGLES::Type GetTextureTypeFromDescriptor(
  const auto usage = static_cast<TextureUsageMask>(desc.usage);
  const auto render_target =
      static_cast<TextureUsageMask>(TextureUsage::kRenderTarget);
-  if (usage == render_target) {
+  const auto is_msaa = desc.sample_count == SampleCount::kCount4;
+  if (usage == render_target && !is_msaa) {
    return TextureGLES::Type::kRenderBuffer;


matanlurey · 2023-10-23T21:22:07Z

@chinmaygarde can you review this as well? thanks.

chinmaygarde · 2023-10-23T21:53:50Z

impeller/renderer/backend/gles/capabilities_gles.cc

+    // We hard-code 4x MSAA, so let's make sure it's supported.
+    GLint value = 0;
+    gl.GetIntegerv(GL_MAX_SAMPLES_EXT, &value);
+    FML_DCHECK(value == 4) << "Unexpected max samples value: " << value << ". "


I am still reading up on the extensions spec but this check seems dodgy. We want to use 4 even when the max is higher. Now per the dashboard, ~83% support the 4 max. But the rest support higher numbers. In those cases, we still need to use 4.

I suggest something like supports_offscreen_msaa_ = value >= 4.

chinmaygarde · 2023-10-23T21:55:01Z

impeller/renderer/backend/gles/formats_gles.h

@@ -185,7 +185,7 @@ constexpr std::optional<GLenum> ToTextureTarget(TextureType type) {
    case TextureType::kTexture2D:
      return GL_TEXTURE_2D;


Nit: Just a [[fallthrough]] here?

chinmaygarde · 2023-10-23T21:57:19Z

impeller/renderer/backend/gles/texture_gles.cc

+  const auto is_msaa = desc.sample_count == SampleCount::kCount4;
+  if (usage == render_target && !is_msaa) {
+    // TODO(matanlurey): MSAA render buffers?
+    // See https://github.com/flutter/flutter/issues/137095.
    return TextureGLES::Type::kRenderBuffer;


I thought we were just getting rid of kRenderBuffer altogether.

We shouldn't try to mix refactors and feature work.

chinmaygarde · 2023-10-23T21:59:34Z

impeller/renderer/backend/gles/render_pass_gles.cc

+  // possible to work around this issue a few different ways (not yet done).
+  //
+  // TODO(matanlurey): See https://github.com/flutter/flutter/issues/137093.
+  if (!is_default_fbo && pass_data.resolve_attachment) {


A missing cap check here for SupportsOffscreenMSAA for paranoia? Instead of the DCHECK for the blit.

We already have cap checks in render target construction. We could add an FML CHECK here but I'd be worried that too many layers of error checking is going to make it hard to find/fix problems.

…e#47030)

…137180) flutter/engine@602b513...d6a48e9 2023-10-24 jonahwilliams@google.com [Impeller] Allocate exact descriptor count, populate in one go. (flutter/engine#47200) 2023-10-24 47866232+chunhtai@users.noreply.github.com [Impeller] Curve components in stroke path use start directions as their initial offsets (flutter/engine#46203) 2023-10-24 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from OUWiWfUxyMyDOlEfA... to YqSO1OByhoexFJSCr... (flutter/engine#47273) 2023-10-24 matanlurey@users.noreply.github.com [Impeller] Enable MSAA for OpenGLES: Take 2. (flutter/engine#47030) 2023-10-24 skia-flutter-autoroll@skia.org Manual roll Dart SDK from 901e92d10627 to 360370ff93b0 (3 revisions) (flutter/engine#47271) 2023-10-24 skia-flutter-autoroll@skia.org Roll Dart SDK from 901e92d10627 to 360370ff93b0 (3 revisions) (flutter/engine#47269) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from OUWiWfUxyMyD to YqSO1OByhoex If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

matanlurey added 2 commits October 17, 2023 09:54

Fix MSAA implementation.

08f4b93

Checkpoint.

1b2b898

matanlurey requested review from chinmaygarde and jonahwilliams October 17, 2023 21:59

github-actions bot added the e: impeller label Oct 17, 2023

matanlurey mentioned this pull request Oct 17, 2023

[Impeller] Continue enabling MSAA for OpenGLES Backend #46688

Closed

chinmaygarde assigned matanlurey Oct 18, 2023

matanlurey added 10 commits October 18, 2023 15:09

Print debugging.

10389b3

Working with serious hacks.

d0f1d7c

Revert impeller/core/formats.cc, it's a NOOP.

99e9c68

Remove unused code.

ce1ffeb

Merge remote-tracking branch 'upstream/main' into engine-impeller-gle…

ad5c75b

…s-msaa-take2

Still working.

5964853

Working mostly.

118ccd8

Revert kDevicePrivate.

64cf63c

if -> dcheck

29fc11a

Discard is back, baby.

29da282

jonahwilliams reviewed Oct 20, 2023

View reviewed changes

impeller/renderer/backend/gles/capabilities_gles.cc Show resolved Hide resolved

matanlurey added 2 commits October 23, 2023 13:17

WIP still not working.

8082acb

Finish and remove logging.

fcefcf6

matanlurey requested a review from jonahwilliams October 23, 2023 21:01

matanlurey marked this pull request as ready for review October 23, 2023 21:01

jonahwilliams reviewed Oct 23, 2023

View reviewed changes

Remove comment.

b7b5333

jonahwilliams approved these changes Oct 23, 2023

View reviewed changes

This was referenced Oct 23, 2023

[Impeller] OpenGLES MSAA Follow-up: Support < OpenGL 3.0 flutter/flutter#137093

Closed

[Impeller] OpenGLES MSAA Follow-up: Investigate MSAA Render Buffers flutter/flutter#137095

Closed

Add TODOs.

c502394

matanlurey commented Oct 23, 2023

View reviewed changes

chinmaygarde reviewed Oct 23, 2023

View reviewed changes

Change capabilities_gles.:

f47324e

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 24, 2023

matanlurey merged commit 09f1d27 into flutter:main Oct 24, 2023
6 of 28 checks passed

matanlurey deleted the engine-impeller-gles-msaa-take2 branch October 24, 2023 18:01

engine-flutter-autoroll mentioned this pull request Oct 24, 2023

Roll Flutter Engine from 602b5131ee4d to 09f1d27f10ad (3 revisions) flutter/flutter#137175

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 24, 2023

09f1d27f1 [Impeller] Enable MSAA for OpenGLES: Take 2. (flutter/engin…

d4dfd8a

…e#47030)

engine-flutter-autoroll mentioned this pull request Oct 24, 2023

Roll Flutter Engine from 602b5131ee4d to d6a48e9963df (6 revisions) flutter/flutter#137180

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 24, 2023

09f1d27f1 [Impeller] Enable MSAA for OpenGLES: Take 2. (flutter/engin…

528739e

…e#47030)

zanderso mentioned this pull request Oct 25, 2023

Impeller/OpenGL framework tests failing after engine roll flutter/flutter#137252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] Enable MSAA for OpenGLES: Take 2. #47030

[Impeller] Enable MSAA for OpenGLES: Take 2. #47030

matanlurey commented Oct 17, 2023 •

edited

Loading

matanlurey commented Oct 18, 2023

matanlurey commented Oct 18, 2023

matanlurey commented Oct 18, 2023

matanlurey commented Oct 23, 2023

jonahwilliams commented Oct 23, 2023

matanlurey commented Oct 23, 2023

jonahwilliams commented Oct 23, 2023

flutter-dashboard bot commented Oct 23, 2023

jonahwilliams Oct 23, 2023

matanlurey Oct 23, 2023

jonahwilliams Oct 23, 2023

matanlurey Oct 23, 2023

jonahwilliams Oct 23, 2023

matanlurey Oct 23, 2023

jonahwilliams left a comment

matanlurey left a comment

matanlurey Oct 23, 2023

matanlurey Oct 23, 2023

matanlurey Oct 23, 2023

matanlurey commented Oct 23, 2023

chinmaygarde Oct 23, 2023 •

edited

Loading

chinmaygarde Oct 23, 2023

chinmaygarde Oct 23, 2023

chinmaygarde Oct 23, 2023

jonahwilliams Oct 23, 2023

chinmaygarde Oct 23, 2023

jonahwilliams Oct 23, 2023

		case PixelFormat::kS8UInt:
		internal_format = GL_DEPTH_STENCIL;

		@@ -185,7 +185,7 @@ constexpr std::optional<GLenum> ToTextureTarget(TextureType type) {
		case TextureType::kTexture2D:
		return GL_TEXTURE_2D;

[Impeller] Enable MSAA for OpenGLES: Take 2. #47030

[Impeller] Enable MSAA for OpenGLES: Take 2. #47030

Conversation

matanlurey commented Oct 17, 2023 • edited Loading

Status

Background

matanlurey commented Oct 18, 2023

matanlurey commented Oct 18, 2023

matanlurey commented Oct 18, 2023

MSAA Enabled

MSAA Disabled

matanlurey commented Oct 23, 2023

jonahwilliams commented Oct 23, 2023

matanlurey commented Oct 23, 2023

jonahwilliams commented Oct 23, 2023

flutter-dashboard bot commented Oct 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahwilliams left a comment

Choose a reason for hiding this comment

matanlurey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matanlurey commented Oct 23, 2023

chinmaygarde Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matanlurey commented Oct 17, 2023 •

edited

Loading

chinmaygarde Oct 23, 2023 •

edited

Loading