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
Blender changes #2281
Blender changes #2281
Conversation
I checked the code. It looks good for me. |
Ok thanks. Also I added a cleanup commit where I refactored the variables into more readable names and corrected the padding and linebreaks. I wrote GL_GET_PROC_ADR in this line because ASSIGN_PROC_ADR wasn't compiling. I'm not sure what is doing in detail. @gonetz @fzurita can you check if it is correct or should be changed? |
ASSIGN_PROC_ADR is for windows systems if I remember correctly where using GL_GET_PROC_ADR will fail for older OpenGL functions. GL_GET_PROC_ADR is probably correct, but you may have it in the incorrect section for it to work in all platforms. |
Yeah, looking closer, you probably want to use I would put it right after this line and remove it from the two other places:
|
Done. The whole PR is in |
BTW, glBlendFuncSeparate is core since OpenGL 2.0 and is supported by GLES 2.0. |
I was just going to say, GL_ARB_blend_func_extended has been core since OpenGL 3.3, which is the minimum required by GLideN64, so there is no need to check for it. For OpenGL ES, the extension is GL_EXT_blend_func_extended, which I don't think is core in any version of GLES. @gonetz I don't think the extension is needed because of the use of glBlendFuncSeparate, but rather the use of things like SRC1_COLOR |
In terms of OpenGL ES support, it looks like the extension is supported on Adreno 5xx and 6xx devices running Android 9 or later, as well as all Nvidia devices (Shield and Pixel C) |
New blender code needs If any of them is core, then I guess there is no need to check for that one. |
Yes that is all core in OpenGL, but not OpenGL ES |
c74c00e
to
bfcad8d
Compare
…ack if extension is not supported.
bfcad8d
to
e661231
Compare
I added some cleanup so that commits so that they can eventually be squashed into two commits, each up to cleanup commit. Feel free to add the fallback commit if we're going to use it. c9f55c1 will need editing if we are to write a fallback for GLES devices. I noticed the following problem with the fallback. It is due to So either we have to deal with it in devices that don't support |
As I see, the glBlendFuncSeparate is core in GLES 2.0, but SRC1_COLOR is not supported by GLES. |
Legacy blending is a regression for GLES3 devices. Thus, I think we need to support the all three ways. @standard-two-simplex could you rewrite the code to fallback into the old blending code? It should be easy: check for config.generalEmulation.enableLegacyBlending and for Context::SeparateBlending and call corresponding blending code. |
Ok, I will give it a go. If it is ok, I will create |
…y the video card. Reset cached blending when separate blending is called and vice versa.
Done. It's working on my PC with and without fallback. @loganmc10 Regarding GLES and GL_EXT_blend_func_extended
|
No, GL_EXT_blend_func_extended requires GLES3
That might be a question for @fzurita , a lot of the GL backend has been abstracted and I can't really remember how the GL headers are pulled in. But generally no, those things should be defined in the GL headers. You shouldn't be re-defining them |
I had to define some of the tokes here. The question was whether I have to define them in a per GL version basis, or if they can be somehow abstracted to work for all versions. |
oh I understand, well that will definitely work for OpenGL. For GLES I'm not 100% sure, it would depend how the GL headers are pulled in, I would just submit it as-is, when someone goes to build for GLES it'll be pretty obvious what needs to be fixed if there is a problem. It's a bit annoying that they add the _EXT for GLES, but I'm sure they have their reasons |
Mmm. If I write GL_SRC1_COLOR_EXT in PC it stops compiling. The extension pages say the following.
GL_EXT_BLEND_FUNC_EXTENDED
If current code fails, perhaps adding
to some GL ES header solves the incompatibility. @fzurita Any idea if this should be necessary? |
Some constants are not defined in all platforms. There is some precedence for defining these constants even though they are not used just to get things to compile. For example:
|
Current code looks ok, but GLES 2 is broken again |
Shader linking error? Do you know if it is GLES2 only or GLES in general? I think the issue might be due to the layout declaration in
|
I'm not sure yet. Probably it is my virtual Android device is broken. Update: yes, the problem is with the layout declaration. Update 2, the fix:
|
I put this PR in https://github.com/gonetz/GLideN64/commits/blender_changes branch. |
I need help with "render 2D in N64 resolution".
Blending works if aux buffer keeps alpha from alpha combiner, as it is with the ordinary blending. With dual blending output alpha comes from the alpha blender. |
I'm not sure what you mean, but I'll explain how dual blending works. Ordinary blending has one output with RGB and A channels. Let's call them
where
but as you may notice, the alpha output is a bit strange. Dual source blending only extends the range of factors that you may plug into
So for example you may perform equation
with an alpha ( Finally,
where you may write a logical alpha output. |
By the way, are you checking the issue Olivier reported or do you know of another place where native rects fails? I have uploaded a local backup branch I had, because code refactoring broke intermediate commits. I have been checking Olivier's issue report and commit 9d842f8 is already creating the issue, which is before alpha blending. As you can see, that commit only makes the previous code use What puzzles me most, is that whatever value I hack into Edit: |
Thanks for the explanation. Now I'll try to explain, what is the problem with native rects "render 2D in N64 resolution" has two phases One. Render texrects into an auxiliary buffer. Blending set off. Thus, the resulted color is the first part of the blending equation: fragColor = fragColor.rgba * SRCFACTOR, because memColor.rgba is set to 0 in the shader. I think, that this method may work with dual blending too. |
So in this step, what do you use as
|
Exactly the same blend mode, which was set to render the texrect. Normal case: Native res case:
Obviously no. That is the problem. |
Ok, I think I get it now. With dual source blending the fragment shader outputs two variables, but only one is written to the color buffer. With blending disabled that would be The simplest way to solve the problem would be to move I hope I'm making more sense now. |
Yes, it is clearer now for me, thanks! However, implementing blending in native res shader is not simple at all - it is a special shader, not included into standard update mechanism. |
It should be possible to output the value of Then you'd have to modify the fragment shader for phase 1 to bind fragColor1 to the second color attachment ( If you feel like you know how to do it, please give it a try. Otherwise, point me to the code where the auxiliary buffer for phase one and the fragment shader for phase 2 are set up, and I'll try to get something, but I'm inexperienced in these regard so I'm not too optimistic. |
Does not it contradict with https://www.khronos.org/opengl/wiki/Blending: Anyway, it all sounds complex. "you'd have to modify the fragment shader for phase 1 to bind fragColor1 to the second color attachment (layout(location = N) OUT lowp vec4 fragColor1;)" is bad. I need too use two versions of shaders and switch between them. Native res rects code is already very complex and thus is not a bug-proof. Such changes will add new level of complexity, so the code will be hard to maintain. Current version with ordinary blending at least does not require special modifications in shaders. If there was a way to switch shaders into native-res compatible mode by changing a uniform parameter, it could save the situation. Unfortunately, we can't use
If you want to check native res texrects code yourself, GLideN64\src\TexrectDrawer.cpp is the place. Special shader for native res created in GLideN64\src\Graphics\OpenGLContext\GLSL\glsl_SpecialShadersFactory.cpp I don't see a good solution yet. Imo, the least evil for now is to disable dual blending when native res texrects are enabled. |
I'm not sure. In theory, the shader from phase one with two attachments wouldn't need dual source blending because blending isn't enabled. Only the shader from phase two needs it, which has only one color attachment. The obvious downside is that the fragment shader would have to be changed whether it's rendering a rect or not.
I agree, things would get complicated and hard to mantain, it might be best to disable dual source blending when native texrects are enabled. The dual source fragment shader is compatible with both modes because it outputs the same |
I finally found time to fix the issue with texrects drawer.
Actually it is not, because fragment alpha is changed by ShaderBlenderAlpha |
For
|
NINTENDO64 logo in Zelda MM uses CVG_SAVE and the logo is invisible in native texrects mode if alpha blending is enabled. |
I tested this PR on a virtual device with GLES 2.0. It works. |
@standard-two-simplex thanks! Another two ancient problems closed. |
You're welcome. And thanks to you for solving the native rects issue! |
This incorporates changes to the blender to emulate cases where memory color is used in the first cycle of a two-cycle blender and some cases where special alpha is blended.
It needs a fallback if extension
GL_EXT_blend_func_extended
orGL_EXT_blend_func_separate
is not supported by the video card. I would appreciate any help with the design of this and supervision of commit 5e55563.Any cleanup suggestions are welcome. I will still try to clean up at least
blender1
andblender2
shader parts.Fixes #585 and #728.