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

renderer_opengl: Add support for custom shaders #4578

Merged
merged 3 commits into from Aug 9, 2019

Conversation

xperia64
Copy link
Contributor

@xperia64 xperia64 commented Jan 18, 2019

Adds support for the majority of Dolphin's glsl post-processing shaders.

Primarily done to add support for different types of Anaglyph 3D, which effectively required adding custom shader support.
For convenience when setting up Anaglyph 3D, the 3D slider can now be adjusted via the configuration menu while the game is running.

A red-cyan Anaglyph Dubois shader is built-in so Anaglyph 3D can be used without further configuration.
Custom shaders can be added in the new shaders folder in the Citra data directory, and
other flavors of Anaglyph can be added in the new shaders/anaglyph directory.

I wasn't quite sure how to structure this, and it's mostly based on my interpretation of the structure of similar components.

Screenshots of the new settings layout:

Stereoscopy modes:
image

Default shader dropdown for Anaglyph
image


This change is Reviewable

@adityaruplaha
Copy link
Contributor

adityaruplaha commented Jan 18, 2019

Does this also add support for custom texture filtering like xBR, HQx?

src/core/settings.cpp Outdated Show resolved Hide resolved
@xperia64
Copy link
Contributor Author

xperia64 commented Jan 18, 2019

@adityaruplaha at the moment no, because I believe that requires custom vertex shader support, while this is only allows custom fragment shaders. A meta-shader file type like libretro's glslp or DraStic's DFX will probably be needed.

@wwylele
Copy link
Member

wwylele commented Jan 18, 2019

Primarily done to add support for different types of Anaglyph 3D, which effectively required adding custom shader support.

For convenience when setting up Anaglyph 3D, the 3D slider can now be adjusted via the configuration menu while the game is running.

Could you separate these into two PRs? Because they have different technical problem and require different group of people to review. You don't want to let them block each other.

src/video_core/renderer_opengl/post_processing_opengl.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_opengl/post_processing_opengl.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_opengl/post_processing_opengl.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_opengl/renderer_opengl.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_opengl/renderer_opengl.cpp Outdated Show resolved Hide resolved
src/core/hle/kernel/shared_page.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_opengl/post_processing_opengl.cpp Outdated Show resolved Hide resolved
#define uint4 uvec4
#define int2 ivec2
#define int3 ivec3
#define int4 ivec4
Copy link
Member

@wwylele wwylele Jan 18, 2019

Choose a reason for hiding this comment

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

I understand that his was copied from dolphin, but since we have no plan supporting HLSL for Direct3D, can we simplify this and remove unnecessary boilerplate?

Copy link
Contributor Author

@xperia64 xperia64 Jan 18, 2019

Choose a reason for hiding this comment

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

This is more for drop-in shader compatibility. This, and the various texture helper functions.

@xperia64
Copy link
Contributor Author

xperia64 commented Jan 19, 2019

@JellyDean446
I have chosen the Dubois method of generating anaglyph as the builtin because while it reduces the color space somewhat, it also substantially reduces eyestrain. The bottom screen is included to keep it in the same color space.

Because this is allows for custom shaders, you can take any of Dolphin's other anaglyph glsl shaders and put them in the Citra/shaders/anaglyph folder if you want to switch back to full color anaglyph.

@wwylele
Copy link
Member

wwylele commented Jan 20, 2019

@JellyDean446 Before we dig into any math thing, did you try wearing 3D glasses and testing it? If not, there is no point to discuss how the screen looks like without 3D glasses as you are not supposed to use it in that way.

@wwylele
Copy link
Member

wwylele commented Jan 20, 2019

@JellyDean446 Then stop talking, thanks. The 3d-fun branch is simply wrong.

@xperia64
Copy link
Contributor Author

xperia64 commented Jan 24, 2019

Screenshot of new option to toggle linear filtering:
image

@tywald
Copy link
Contributor

tywald commented Jan 29, 2019

Testing Linear Filter feature on the AppVeyor build. It seems when you uncheck filtering and then upscale it appears there is still some filtering applied. At native 240p it's definitely nearest as expected.

The screenshots below show a comparison between 240p nearest, 240p linear and 4x upscaling(960p) nearest. The game is Dragon Quest VII: Fragments of the Forgotten Past.

Native, nearest:
240p-nearest png

Native, linear:
240p-linear

4x nearest:
4x-nearest

The portrait on the 4x nearest screenshot looks very close to the 240p linear. Except for the artifacts(horizontal & vertical lines around the frames).

@xperia64
Copy link
Contributor Author

xperia64 commented Jan 29, 2019

@tywald the nearest option only affects the final texture, not whatever filtering the games may be internally requesting.

@xperia64 xperia64 force-pushed the shaders branch 2 times, most recently from 8f2842b to c990915 Compare Mar 24, 2019
@B3n30
Copy link
Contributor

B3n30 commented Apr 23, 2019

please resolve conflicts

@tywald
Copy link
Contributor

tywald commented Jun 10, 2019

Hm, it seems this PR generates this in the log:

Render.OpenGL <Critical> video_core/renderer_opengl/renderer_opengl.cpp:DebugHandler:696: API ERROR 1281: GL_INVALID_VALUE error generated. Handle does not refer to a shader or program object.

Doesn't appear on Nightly.

However, I haven't encountered any issue in games.

@B3n30
Copy link
Contributor

B3n30 commented Jul 6, 2019

This PR is in canary since 1 month and so far no issues where reported besides the Critical LOG? How should we proceed with that PR?

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 commented Jul 8, 2019

Please resolve conflicts

xperia64 added 2 commits Jul 8, 2019
Change 3D slider in-game

Change shaders while game is running

Move shader loading into function

Disable 3D slider setting when stereoscopy is off

The rest of the shaders

Address review issues

Documentation and minor fixups

Forgot clang-format

Fix shader release on SDL2-software rendering

Remove unnecessary state changes

Respect 3D factor setting regardless of stereoscopic rendering

Improve shader resolution passing

Minor setting-related improvements

Add option to toggle texture filtering

Rebase fixes
@tywald
Copy link
Contributor

tywald commented Jul 8, 2019

Confirmed, no more critical logs.

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 commented Jul 24, 2019

I just glanced over the code, and I'm not sure if I like the idea of using straight string comparisons for the builtin shaders. The same string is repeated too many times in the code, which makes it more error-prone. I think it would be more acceptable to just use those two default shaders when the setting field is empty, instead of a certain string value.

@B3n30 B3n30 merged commit 8131bd3 into citra-emu:master Aug 9, 2019
3 checks passed
@xperia64 xperia64 deleted the shaders branch May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants