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

Assets/icon: Rm redundant PNG file #33565

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

Conversation

ugtthis
Copy link
Contributor

@ugtthis ugtthis commented Sep 16, 2024

selfdrive/assets/offroad/icon_openpilot.png - This file gets deleted

  • size: 43KB
  • Icon used only in "Enable openpilot" settings toggle
  • Current size when rendered 80x80 pixels - Refer to selfdrive/ui/qt/widgets/controls.cc
    icon_pixmap = QPixmap(icon).scaledToWidth(80, Qt::SmoothTransformation);

selfdrive/assets/img_chffr_wheel.png

  • size: 17KB
  • Icon only used in onroad UI(icon in top right)
  • Current size when rendered 144x144 pixels - Refer to selfdrive/ui/qt/onroad/buttons.h
    const int btn_size = 192;
    const int img_size = (btn_size / 4) * 3;

I removed icon_openpilot.png because its 532x532 px size is unnecessary for a small 80x80 px icon displayed on a 2160x1080 screen. img_chffr_wheel.png(266x266 px) is used for a larger 144x144 px icon but has a smaller file size. Quality of icon when using the smaller file size as the replacement, looks similar/the same to how the old icon rendered, as seen in video below.


Comparing files

png-compare



Comparison video

comparing-old-and-new-icon.mp4

Copy link
Contributor

github-actions bot commented Sep 16, 2024

UI Preview

onroad : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
All Screenshots

@sshane
Copy link
Contributor

sshane commented Sep 16, 2024

@deanlee know why the DM crosshair changes slightly here? Also can you add a screenshot of the toggles pane? That would help us verify this PR!

@VirtuallyChris
Copy link
Contributor

Maybe it's time to rename the chffr wheel? That's such an old reference!

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