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

[Impeller] Float samplers can get re-ordered compared to SkSL #118847

Closed
jonahwilliams opened this issue Jan 19, 2023 · 6 comments · Fixed by flutter/engine#39077
Closed

[Impeller] Float samplers can get re-ordered compared to SkSL #118847

jonahwilliams opened this issue Jan 19, 2023 · 6 comments · Fixed by flutter/engine#39077
Assignees
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list

Comments

@jonahwilliams
Copy link
Member

When i looked at this last I thought that this was handled correctly via our uniform mapping, but @zanderso has shared a few places where that isn't the case. Need to investigate what is different about these and whether or not we should revist uniform re-mapping for floats.

@jonahwilliams jonahwilliams added engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list e: impeller Impeller rendering backend issues and features requests labels Jan 19, 2023
@jonahwilliams jonahwilliams self-assigned this Jan 19, 2023
@github-project-automation github-project-automation bot moved this to 🤔 Needs Triage in Impeller Jan 24, 2023
@chinmaygarde chinmaygarde moved this from 🤔 Needs Triage to ✅ Done in Impeller Jan 24, 2023
@zanderso
Copy link
Member

I believe there's still a dependence on use order. @jonahwilliams it shows up in the glowing text example in Impeller on Metal that I sent you awhile back. Maybe it's only cropped up after flutter/engine#39147 and flutter/engine#39198 --- I haven't bisected yet.

@zanderso zanderso reopened this Jan 29, 2023
@github-project-automation github-project-automation bot moved this from ✅ Done to 🤔 Needs Triage in Impeller Jan 29, 2023
@jonahwilliams
Copy link
Member Author

Those PRs only adjusted sampler use order - are you seeing float re-ordering? I'd expect them to be re-ordered but the mapping data should account for it. We don't correctly handle uniforms getting optimized out yet.

@zanderso
Copy link
Member

Hmm. Shoot. This isn't repro'ing for me anymore. Sorry for the false alarm. I think I probably wasn't being sufficiently paranoid when swapping among a few different framework and engine versions...

@github-project-automation github-project-automation bot moved this from 🤔 Needs Triage to ✅ Done in Impeller Jan 29, 2023
@zanderso
Copy link
Member

I figured out that this wasn't repro'ing for me earlier today because I was swapping the declaration order of uniforms in the shader, but not swapping the indices used to set the uniforms in the Dart code. That is, the uniforms were set by the Dart code in use-order, and so changing the declaration order in the shader didn't trigger the issue. So, sadly I have to re-open this =(

@zanderso zanderso reopened this Jan 30, 2023
@github-project-automation github-project-automation bot moved this from ✅ Done to 🤔 Needs Triage in Impeller Jan 30, 2023
@chinmaygarde chinmaygarde moved this from 🤔 Needs Triage to 🐞 Bugs in Impeller Feb 2, 2023
@chinmaygarde chinmaygarde added this to the Impeller on iOS milestone Feb 2, 2023
@github-project-automation github-project-automation bot moved this from 🐞 Bugs to ✅ Done in Impeller Feb 3, 2023
@jonahwilliams
Copy link
Member Author

Fixed for real this time!

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants