Skip to content

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
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