Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@zanderso
Copy link
Member

No description provided.

Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM

@zanderso zanderso added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2023
@bdero
Copy link
Member

bdero commented Mar 28, 2023

Instead of doing partial diffs on CI, would it make sense to just generate a blessed file and literally run diff ./impeller/tools/malioc.json [after.json]?

@auto-submit auto-submit bot merged commit 9025d98 into flutter:main Mar 28, 2023
@zanderso zanderso deleted the malioc-diff-shader-rm branch March 28, 2023 19:42
@zanderso
Copy link
Member Author

Instead of doing partial diffs on CI, would it make sense to just generate a blessed file and literally run diff ./impeller/tools/malioc.json [after.json]?

Yeah, there are a couple of different directions we could go. One thought I had was that when there is a change in performance in an existing shader, then having the prettier message is useful. But if that's not actually the case, then we can tear out a bunch of code. OTOH, if it is useful, then I'd go even further in the same direction and pull the human readable display names and verbose descriptions out of the malioc json output as well.

@bdero
Copy link
Member

bdero commented Mar 28, 2023

Having a readable interpretation seems valuable to me. Perhaps we could do both and get the best of both worlds. The primary value in printing the full diff for me would be the ability to just copy it from CI and apply it using a patch tool in cases where I forgot (or can't) run malioc locally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants