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

Add global shader list support to s2d #1265

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Yanrishatum
Copy link
Contributor

Introduces ability to add global shaders to 2D rendering pipeline.

Rationale: Allows users to add shaders to multiple rendered objects without having to add same shader to all of them, or apply a shader that have to be always present on all 2D objects. This reduces the amount of tedious shader management.
Use-case examples:

  • Custom global filtering. In my case bilinear filtering that mimics nearest neighbor but offers reasonably antialiased transition between texels on rotated sprites.
  • (From discord) Adding custom pass outputs and shader that outputs to it.
  • Shader that is applied to all object children, simplifying the shader management code, as instead of having to add shader to everything in the object tree and tracking that, it's possible to just add shader as global one during root object rendering, and removing it afterwards.

Adds the following API:

  • RenderContext.addGlobalShader(shader)
  • RenderContext.removeGlobalShader(shader)

A few notes:

  • Is there any reason for currentShaders to even exist? It seems fairly redundant, as initShaders only ever called with baseShaderList, and thus it can never be anything other than baseShaderList. Some legacy code leftovers?
  • Shaders are added without sort, as they are sorted anyway by Cache.compileRuntimeShader
  • It probably would be better to rename baseShaderList to globalShaderList.

@skial skial mentioned this pull request Jan 20, 2025
1 task
h2d/RenderContext.hx Outdated Show resolved Hide resolved
@@ -781,7 +830,7 @@ class RenderContext extends h3d.impl.RenderContext {
flush();
var shaderChanged = needInitShaders, paramsChanged = false;
var objShaders = obj.shaders;
var curShaders = currentShaders.next;
var curShaders = currentShadersTail.next;
Copy link

Choose a reason for hiding this comment

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

think this needs a null check

Suggested change
var curShaders = currentShadersTail.next;
var curShaders = currentShadersTail?.next;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because it's never supposed to be null. Tail points at the last global shader (tail.next points at object-specific shaders), and you should not be able to remove base shader from the global shader list, thus tail is never null.

Copy link

Choose a reason for hiding this comment

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

🤷 idk i had to change this to make it work locally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you provide an repro sample?

@ncannasse
Copy link
Member

Shouldn't we have instead something like push/pop mechanism so we can use it during rendering ?

@Yanrishatum
Copy link
Contributor Author

Shouldn't we have instead something like push/pop mechanism so we can use it during rendering ?

Elaborate? This is pretty much how it currently behaves. Nothing prevents user from adding shaders mid-rendering and then removing them. Or you mean somehting along the lines push/popGlobalShaderList where user would provide a list of shaders that would replace current list (with exception of base shader) and have them on a stack?

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 this pull request may close these issues.

3 participants