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

[Impeller] Skip TextRotated golden test. #40818

Merged
merged 1 commit into from
Mar 31, 2023
Merged

[Impeller] Skip TextRotated golden test. #40818

merged 1 commit into from
Mar 31, 2023

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Mar 31, 2023

This test ends up with small differences depending on which mac host it runs on and will end up blocking rollers.

This test ends up with small differences depending on which mac host it runs on and will end up blocking rollers.
@dnfield dnfield requested review from bdero, zanderso and gaaclarke March 31, 2023 05:27
Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

The insight from the Skia team was that the golden tests were to be verified out of band by the triagers and not in the critical path for rollers. Perhaps we should follow a similar approach. The test is unlikely to ever offer pixel perfect reproducibility but disabling it gives us nothing. Disabling the test entirely to appease the roller is perhaps throwing the baby out with the bathwater.

LGTM if this is to get the rollers going but we should consider holding doing this and instead stop the rollers from tripping on goldens.

@chinmaygarde chinmaygarde changed the title Skip TextRotated golden test [Impeller] Skip TextRotated golden test. Mar 31, 2023
@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 31, 2023
@dnfield
Copy link
Contributor Author

dnfield commented Mar 31, 2023

/cc @Piinks - right now the golden checker blocks submit AFAIU by keeping a check as pending until images are triaged. I think this works reasonably well for the framework, but not as well for this case. Maybe we can brainstorm some ways to make this be non-blocking for rollers in the engine.

@dnfield
Copy link
Contributor Author

dnfield commented Mar 31, 2023

(and yes, the goal of this PR is to make sure rollers don't get stuck on golden approvals)

@gaaclarke
Copy link
Member

@chinmaygarde @dnfield there is parameter when we submit to skia gold about how strict we want the golden test to be. We could potentially lower the threshold.

@Piinks
Copy link
Contributor

Piinks commented Mar 31, 2023

We could potentially lower the threshold.

I think on some platforms this is already done (web I think?) via the fuzzy matching algorithm. So we could add it for the mac platform probably.

@mdebbar knows better the config for the engine Gold instance

@chinmaygarde
Copy link
Member

I'd rather just follow the Skia GPU teams recommendations on their use of Gold and have this be out of band.

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 e: impeller
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants