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

Added support for structs in shader uniforms #6298

Merged
merged 2 commits into from
Feb 29, 2020
Merged

Added support for structs in shader uniforms #6298

merged 2 commits into from
Feb 29, 2020

Conversation

jnsmalm
Copy link
Contributor

@jnsmalm jnsmalm commented Dec 28, 2019

Description of change

With this change we can set structs in shader uniforms the same way we can set it using WebGL directly.

shader.uniforms["material.color"] = [1, 1, 1];
shader.uniforms["lights[1].position"] = [0, 1, 0];
Pre-Merge Checklist
  • [ x] Tests and/or benchmarks are included
  • Documentation is changed or added
  • [x ] Lint process passed (npm run lint)
  • [x ] Tests passed (npm run test)

@codecov-io
Copy link

codecov-io commented Dec 28, 2019

Codecov Report

Merging #6298 into dev will decrease coverage by 1.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev    #6298     +/-   ##
=========================================
- Coverage   78.33%   77.23%   -1.1%     
=========================================
  Files          57      178    +121     
  Lines        2820     9511   +6691     
=========================================
+ Hits         2209     7346   +5137     
- Misses        611     2165   +1554
Impacted Files Coverage Δ
packages/core/src/shader/Program.js 93.93% <100%> (ø)
...ages/core/src/shader/utils/generateUniformsSync.js 86.2% <100%> (ø)
packages/canvas/canvas-renderer/src/canvasUtils.js 2.35% <0%> (-37.86%) ⬇️
...nvas/canvas-graphics/src/CanvasGraphicsRenderer.js 27.56% <0%> (-7.96%) ⬇️
packages/text-bitmap/src/BitmapFontLoader.js 75% <0%> (-3.44%) ⬇️
packages/text-bitmap/src/BitmapFontData.js
packages/text-bitmap/src/formats/index.js
packages/text-bitmap/src/formats/XMLFormat.js
packages/text-bitmap/src/BitmapFont.js
packages/text-bitmap/src/formats/TextFormat.js
... and 129 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67ff508...34b6f75. Read the comment docs.

@GoodBoyDigital
Copy link
Member

this is really cool!

@jnsmalm
Copy link
Contributor Author

jnsmalm commented Jan 7, 2020

:) What do you other guys think, could we add this to next pixi version?

@bigtimebuddy bigtimebuddy added this to the v5.3.0 milestone Jan 10, 2020
@bigtimebuddy
Copy link
Member

I have no problem with this feature. There are some conflicts because we just finished the TypeScript convert of core, and @GoodBoyDigital has a PR right now that will also conflict. If you can have a little patience, we can work on getting this in.

@ivanpopelyshev
Copy link
Collaborator

Yep, lets merge it after @GoodBoyDigital PR.

@jnsmalm
Copy link
Contributor Author

jnsmalm commented Feb 3, 2020

Cool, should I try to resolve these conflicts or should I just leave it as is?

@bigtimebuddy
Copy link
Member

@jnsmalm wait until #6369 is merged and then if you could resolve conflicts after, that'd be awesome.

@jnsmalm
Copy link
Contributor Author

jnsmalm commented Feb 28, 2020

I resolved the conflicts, so it should be ready now :)

@bigtimebuddy bigtimebuddy merged commit f0a66e1 into pixijs:dev Feb 29, 2020
@bigtimebuddy
Copy link
Member

Thank you @jnsmalm 🎉

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.

None yet

5 participants