Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Wrong multisample quality detected #9269

Open
BarberDucky opened this issue Mar 16, 2023 · 14 comments · May be fixed by #9336
Open

Bug: Wrong multisample quality detected #9269

BarberDucky opened this issue Mar 16, 2023 · 14 comments · May be fixed by #9336
Assignees

Comments

@BarberDucky
Copy link

BarberDucky commented Mar 16, 2023

Current Behavior

Property renderer.multisample returns 4 (MSAA_QUALITY.MEDIUM).

Expected Behavior

WebGL Report reports the framebuffer parameter Max Samples to be 8. It is therefore expected that the property renderer.multisample should return 8 (MSAA_QUALITY.HIGH).

Steps to Reproduce

On a device which has framebuffer parameter Max Samples equal to 8, simply read the renderer.multisample property.

Environment

  • pixi.js version: 7.1.4
  • Browser & Version: Chrome 111.0.5563.64
  • OS & Version: Ubuntu 22.04

Possible Solution

The code for MultisampleSystem determines the multisample quality by accessing the gl.SAMPLES constant, both for WebGL1 and WebGL2. Same thing for FramebufferSystem on one occasion. However, WebGL2 has an additional constant used for multisampling — gl.MAX_SAMPLES. This is the one that WebGL Report uses. These two values are different, first one being 4, second being 8. I couldn't find what these two constants represent exactly and what is the difference between them.

If it is decided that gl.MAX_SAMPLES is the one to be used, the solution would probably be to read this value instead of gl.SAMPLES for WebGL2.

@SuperSodaSea
Copy link
Member

glGet OpenGL ES 2.0 man page
GL_SAMPLES
returns a single integer value indicating the coverage mask size of the currently bound framebuffer. See glSampleCoverage.

glGet OpenGL ES 3.0 man page
GL_MAX_SAMPLES
returns one value. The value indicates the maximum supported number of samples for multisampling. The value must be at least 4. See glGetInternalformativ.

In my opinion renderer.multisample should be the number of samples of the default framebuffer (for example, 0 when using canvas.getContext('webgl', { antialias: false }) and 4 when { antialias: true }), not the max number of samples supported, so in my opinion using gl.getParameter(gl.SAMPLES) is right. (You can create your own new 8-sample framebuffer, but the sample count of default framebuffer seems to be non-customizable.)

@dev7355608
Copy link
Collaborator

@SuperSodaSea is correct. renderer.multisample is used when we want to create render textures and filters of the same quality as the default framebuffer (canvas).

@BarberDucky
Copy link
Author

Render textures can be created with a higher multisample rate (as per the above example when using gl.MAX_SAMPLES). How can I access this parameter through pixi in order to get the highest possible antialiasing quality?

@SuperSodaSea
Copy link
Member

Set renderTexture.framebuffer.multisample to MSAA_QUALITY.HIGH and PixiJS will automatically choose the maximum possible number of samples.

this.msaaSamples = gl.getInternalformatParameter(gl.RENDERBUFFER, gl.RGBA8, gl.SAMPLES);

protected detectSamples(samples: MSAA_QUALITY): MSAA_QUALITY
{
const { msaaSamples } = this;
let res: number = MSAA_QUALITY.NONE;
if (samples <= 1 || msaaSamples === null)
{
return res;
}
for (let i = 0; i < msaaSamples.length; i++)
{
if (msaaSamples[i] <= samples)
{
res = msaaSamples[i];
break;
}
}
if (res === 1)
{
res = MSAA_QUALITY.NONE;
}
return res;
}

@SuperSodaSea
Copy link
Member

SuperSodaSea commented Mar 16, 2023

(There are some devices supporting 16-sample framebuffer, so maybe you can try to set 16 instead of MSAA_QUALITY.HIGH (8).)

@BarberDucky
Copy link
Author

The line gl.getInternalformatParameter(gl.RENDERBUFFER, gl.RGBA8, gl.SAMPLES); returns [16, 8, 4] on a Samsung A13. On the other hand, gl.MAX_SAMPLES gives 4. When I set the quality to 8 or 16 explicitly for the render texture, nothing gets rendered and I get this warning stack:

[.WebGL-0x50c64280]GL ERROR :GL_INVALID_VALUE : glRenderbufferStorageMultisample: samples too large
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glClear: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glBlitFramebufferCHROMIUM: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glDrawElements: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glDrawElements: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glDrawElements: framebuffer incomplete
[.WebGL-0x50c64280]GL ERROR :GL_INVALID_FRAMEBUFFER_OPERATION : glDrawElements: framebuffer incomplete

Works fine for 4.

What do the values [16, 8, 4] even mean then?

@SuperSodaSea
Copy link
Member

glGetInternalformativ OpenGL ES 3.0 man page
For every other accepted internalformat, the maximum value in GL_SAMPLES is guaranteed to be at least GL_MAX_SAMPLES.

OpenGL ES Spec 4.4.2.2 Required Renderbuffer Formats
Implementations must support creation of renderbuffers in these required formats with up to the value of MAX_SAMPLES multisamples...

So I think the return value in gl.getInternalformatParameter(gl.RENDERBUFFER, gl.RGBA8, gl.SAMPLES) can be greater than gl.getParameter(gl.MAX_SAMPLES), but using values greater than gl.getParameter(gl.MAX_SAMPLES) in gl.renderbufferStorageMultisample() is invalid. We should ignore these values.

@dev7355608
Copy link
Collaborator

gl.getInternalformatParameter(gl.RENDERBUFFER, gl.RGBA8, gl.SAMPLES) should return the valid samples for renderbuffers with internel format RGBA8.

OpenGL ES 3.0 - 6.1.15 Internal Format Queries

If pname is SAMPLES, the sample counts supported for internalformat and target are written into params, in descending numeric order. Only positive values are returned.

It also says:

Since multisampling is not supported for signed and unsigned integer internal
formats, the value of NUM_SAMPLE_COUNTS will be zero for such formats.
For every other accepted internalformat, the value of NUM_SAMPLE_COUNTS
is guaranteed to be at least one, and the maximum value in SAMPLES is guaranteed
to be at least the value of MAX_SAMPLES.

So all renderbuffers except those with integer format support at minimum MAX_SAMPLES samples, but not necessarily exactly MAX_SAMPLES. It doesn't mean that using a higher samples value is invalid. I think the idea is that for noninteger framebuffers you can always you the value return by querying MAX_SAMPLES without checking getInternalformatParameter for the specific internal format. 4.4.2.2 states that all values up to MAX_SAMPLES must be supported, which would mean that MAX_SAMPLES has to be in the list. But technically wouldn't that mean that getInternalformatParameter had to return something like [8, 7, 6, 5, 4, 3, 2, 1, 0] instead of [8, 4]?

@BarberDuckyFazi
Copy link

So in any way, MAX_SAMPLES should be incorporated when Pixi picks the multisample rate automatically, right?

@dev7355608
Copy link
Collaborator

@BarberDucky What does gl.getInternalformatParameter(gl.RENDERBUFFER, gl.DEPTH24_STENCIL8, gl.SAMPLES) return on your Samsung? Maybe there's a stencil attachment involved, and for some reason stencil attachments only support 4 samples at most on this device.

@BarberDuckyFazi
Copy link

@dev7355608 It returns [16, 8, 4], same as for gl.getInternalformatParameter(gl.RENDERBUFFER, gl.RGBA8, gl.SAMPLES).

@dev7355608
Copy link
Collaborator

It must be a driver bug then. [16, 8, 4] looks wrong. Usually it's all powers of 2 from 1 to whatever the max is. Does 2 work? Technically it shouldn't. Override msaaSamples to test this:

renderer.framebuffer.msaaSamples = [4, 2, 1];
renderTexture.multisample = 2;

And what does this return?:

gl = renderer.gl;
samples= {};
for (const internalFormat of ['R8', 'RG8', 'RGB8', 'RGB565',
    'RGBA4', 'RGB5_A1', 'RGBA8', 'RGB10_A2', 'SRGB8_ALPHA8',
    'DEPTH_COMPONENT16', 'DEPTH_COMPONENT24', 'DEPTH_COMPONENT32F',
    'DEPTH24_STENCIL8', 'DEPTH32F_STENCIL8', 'STENCIL_INDEX8'])
{
    samples[internalFormat] = Array.from(gl.getInternalformatParameter(
        gl.RENDERBUFFER, gl[internalFormat], gl.SAMPLES));
}
samples

@BarberDuckyFazi
Copy link

So for the first part, the app works, but it seems that there is no visible difference between multisamples 2 and 4. I would like to add that I get [16, 8, 4] on two different Samsung devices. Furthermore, I get [8, 4, 2] on my desktop. Either way, no value 1.

As for the second part:

DEPTH24_STENCIL8: [16, 8, 4]
DEPTH32F_STENCIL8: [16, 8, 4]
DEPTH_COMPONENT16: [16, 8, 4]
DEPTH_COMPONENT24: [16, 8, 4]
DEPTH_COMPONENT32F: [16, 8, 4]
R8: [16, 8, 4]
RG8: [16, 8, 4]
RGB5_A1: [16, 8, 4]
RGB8: [16, 8, 4]
RGB10_A2: [16, 8, 4]
RGB565: [16, 8, 4]
RGBA4: [16, 8, 4]
RGBA8: [16, 8, 4]
SRGB8_ALPHA8: [16, 8, 4]
STENCIL_INDEX8: [16, 8, 4, 1]

@dev7355608
Copy link
Collaborator

Not sure if this is the relevant code, but I found this: here MAX_SAMPLES is used to check whether the samples are too large:
https://chromium.googlesource.com/chromium/src/gpu/+/refs/heads/main/command_buffer/service/gles2_cmd_decoder.cc#9510
But this code seems to suggest that the devs understood that MAX_SAMPLES is supposed to be the minimum of the maximum possible samples for each internal format: https://chromium.googlesource.com/chromium/src/gpu/+/refs/heads/main/command_buffer/service/gles2_cmd_decoder.cc#4340
But it seems that it doesn't filter out samples above MAX_SAMPLES when getInternalformatParameter is called. So the array returned by it contains samples that should work, but it doesn't allow you to use it because they are above MAX_SAMPLES. I think this is bugged.

I guess we can just filter out samples above MAX_SAMPLES. I don't think it will make a difference for most devices anyway, because I expect that the valid samples for all internal formats is the same and the maximum is MAX_SAMPLES.

@dev7355608 dev7355608 linked a pull request Apr 4, 2023 that will close this issue
4 tasks
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 a pull request may close this issue.

4 participants