Skip to content

Conversation

@luanpotter
Copy link
Member

@luanpotter luanpotter commented Aug 25, 2024

Description

Refactor shader uniform binding to support shader arrays.

This also decouples the whole shader and uniform byte handling code (that we should definitely test) from the flutter_gpu primitives that are impossible to mock (base native classes).

This adds tests that ensure the arrays are bound as they should - however the underlying flutter_gpu code does not seem to work. See this PR for a test of using this to support an arbitrary number of lights.

Either way, we can merge this as is as this refactors the underlying structure to support arrays when ready, and make it more testable as well.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@luanpotter luanpotter changed the title feat: Refactor shader uniform binding to support shader arrays feat: Refactor shader uniform binding to support shader arrays [flame_3d] Sep 11, 2024
@luanpotter luanpotter marked this pull request as ready for review September 12, 2024 21:01
super.onGameResize(size);

this.size = size * .4;
this.size = size * 0.4;
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated DCM

_vertexShader.resource,
_fragmentShader.resource,
_vertexShader.compile().resource,
_fragmentShader.compile().resource,
Copy link
Member Author

@luanpotter luanpotter Sep 12, 2024

Choose a reason for hiding this comment

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

I'm open to any other cleaner ways to make this testable

@spydon
Copy link
Member

spydon commented Sep 12, 2024

Have you reported that it doesn't work in flutter_gpu?

@luanpotter
Copy link
Member Author

I've been talking with @bdero about it; he suggested using DeviceBuffer instead of HostBuffer but I haven't had time to try it yet.

Either way, I believe the Flame-facing API should be independent of this (though I def want to completely overhaul it to be type safe, the current one is reliant on strings).

luanpotter added a commit that referenced this pull request Sep 13, 2024
Support skeletal animation basics.

Sadly we cannot support arrays yet - see [this
PR](#3282).
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm!

@luanpotter luanpotter merged commit 0c0e8f2 into flame_3d Sep 21, 2024
@luanpotter luanpotter deleted the luan.shader-arrays branch September 21, 2024 00:18
luanpotter added a commit that referenced this pull request Oct 15, 2024
Support skeletal animation basics.

Sadly we cannot support arrays yet - see [this
PR](#3282).
luanpotter added a commit that referenced this pull request Oct 15, 2024
…_3d] (#3282)

Refactor shader uniform binding to support shader arrays.

This also decouples the whole shader and uniform byte handling code
(that we should definitely test) from the flutter_gpu primitives that
are impossible to mock (base native classes).

This adds tests that ensure the arrays are bound as they should -
however the underlying flutter_gpu code does not seem to work. See [this
PR](#3284) for a test of using
this to support an arbitrary number of lights.

Either way, we can merge this as is as this refactors the underlying
structure to support arrays when ready, and make it more testable as
well.
luanpotter added a commit that referenced this pull request Dec 11, 2024
Support skeletal animation basics.

Sadly we cannot support arrays yet - see [this
PR](#3282).
luanpotter added a commit that referenced this pull request Dec 11, 2024
…_3d] (#3282)

Refactor shader uniform binding to support shader arrays.

This also decouples the whole shader and uniform byte handling code
(that we should definitely test) from the flutter_gpu primitives that
are impossible to mock (base native classes).

This adds tests that ensure the arrays are bound as they should -
however the underlying flutter_gpu code does not seem to work. See [this
PR](#3284) for a test of using
this to support an arbitrary number of lights.

Either way, we can merge this as is as this refactors the underlying
structure to support arrays when ready, and make it more testable as
well.
luanpotter added a commit that referenced this pull request Dec 11, 2024
Support skeletal animation basics.

Sadly we cannot support arrays yet - see [this
PR](#3282).
luanpotter added a commit that referenced this pull request Dec 11, 2024
…_3d] (#3282)

Refactor shader uniform binding to support shader arrays.

This also decouples the whole shader and uniform byte handling code
(that we should definitely test) from the flutter_gpu primitives that
are impossible to mock (base native classes).

This adds tests that ensure the arrays are bound as they should -
however the underlying flutter_gpu code does not seem to work. See [this
PR](#3284) for a test of using
this to support an arbitrary number of lights.

Either way, we can merge this as is as this refactors the underlying
structure to support arrays when ready, and make it more testable as
well.
nickf2k pushed a commit to nickf2k/flame that referenced this pull request Nov 16, 2025
Support skeletal animation basics.

Sadly we cannot support arrays yet - see [this
PR](flame-engine#3282).
nickf2k pushed a commit to nickf2k/flame that referenced this pull request Nov 16, 2025
…_3d] (flame-engine#3282)

Refactor shader uniform binding to support shader arrays.

This also decouples the whole shader and uniform byte handling code
(that we should definitely test) from the flutter_gpu primitives that
are impossible to mock (base native classes).

This adds tests that ensure the arrays are bound as they should -
however the underlying flutter_gpu code does not seem to work. See [this
PR](flame-engine#3284) for a test of using
this to support an arbitrary number of lights.

Either way, we can merge this as is as this refactors the underlying
structure to support arrays when ready, and make it more testable as
well.
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