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

Use opaque colors #689

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

chriadam
Copy link
Contributor

Alpha blending is expensive. Reducing alpha blending by using opaque colors reduces GPU work, and thus swap time.

@chriadam chriadam force-pushed the chriadam/brief-page-opaque-colors branch from 9f0fe9d to 153b592 Compare January 29, 2024 07:22
Q_OBJECT
QML_NAMED_ELEMENT(Theme)
QML_SINGLETON
Q_OBJECT
Copy link
Contributor

Choose a reason for hiding this comment

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

tabs look wonky

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code which forms part of the output needs tabs, whereas the python code needs spaces. Hence the discrepancy. What is in the PR is correct, I believe.

Copy link
Contributor

@DanielMcInnes DanielMcInnes left a comment

Choose a reason for hiding this comment

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

lgtm

@chriadam
Copy link
Contributor Author

Putting this on hold for now - asked Serj instead if he could just provide opaque colours for all elements. Hopefully he agrees. If not, we can go down this PR path (and maybe provide a Theme.blendColors(elementColor, surfaceColor) mixer function, which calculates the blending for us, rather than manually providing opaque theme colour values).

@chriadam chriadam force-pushed the chriadam/brief-page-opaque-colors branch from 153b592 to 3846f17 Compare February 1, 2024 04:10
@chriadam chriadam changed the title Use opaque colors on the Brief page Use opaque colors Feb 1, 2024
@chriadam
Copy link
Contributor Author

chriadam commented Feb 1, 2024

Serj provided some opaque colours, but they didn't match the existing colour scheme (it might be intentional or might be a mistake) and a variety of colours were missing, unfortunately. I've added a set of colours which match the previous design, and covers all of the colours we use in our code. I have asked Serj whether these colours need any adjustments.

@chriadam chriadam force-pushed the chriadam/brief-page-opaque-colors branch 2 times, most recently from 3dc72d6 to 7fa0639 Compare May 30, 2024 06:50
Alpha blending is expensive.  Reducing alpha blending by using
opaque colors reduces GPU work, and thus `swap` time.
@chriadam chriadam force-pushed the chriadam/brief-page-opaque-colors branch from 7fa0639 to 6f599b2 Compare May 30, 2024 09:23
@MikeTrahearn-Qinetic
Copy link
Contributor

The ability to define opaque colors is dependent on how well defined the colors are in the Figma Design System.
Where there are a small number of common colors set on top of a fairly common set of background colors, it is possible to define sets of pre-multiplied alpha colours - one for each Dark and Light mode. Having gone through this exact scenario on a previous project, this is less than trivial, unless you build it in to the Design System from the beginning.

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