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

[BUG]: Bar cannot display UWP Icons (PR with fix already exists) #1226

Open
davor-skontra opened this issue Jan 11, 2025 · 12 comments
Open

[BUG]: Bar cannot display UWP Icons (PR with fix already exists) #1226

davor-skontra opened this issue Jan 11, 2025 · 12 comments
Labels
bug Something isn't working i-will-probably-work-on-this I'll work on this when I get time, but if someone beats me to it, thank you! komorebi-bar Related to the komorebi-bar crate

Comments

@davor-skontra
Copy link

Summary

UWP -> Universal Windows Platform

I created a PR in LGUG2>/window-icons repo that merges upstream changes with a fix.

Thought I'd also create this bug report so the PR doesn't go unnoticed.

Version Information

Any version since bar was introduced.

Komorebi Configuration

{
  "$schema": "https://raw.githubusercontent.com/LGUG2Z/komorebi/v0.1.32/schema.bar.json",
  "monitor": {
    "index": 0,
    "work_area_offset": {
      "left": 0,
      "top": 35,
      "right": 0,
      "bottom": 35
    }
  },
  "position": {
    "end": {
      "x": 2560,
      "y": 35,
    }
  },
  "font_family": "JetBrains Mono",
  "theme": {
    "palette": "Base16",
    "name": "TokyoNightStorm",
    "accent": "Base0D"
  },
  "left_widgets": [
    {
      "Komorebi": {
        "workspaces": {
          "enable": true,
          "hide_empty_workspaces": true
        },
        "layout": {
          "enable": true
        },
        "focused_window": {
          "enable": true,
          "show_icon": true
        }
      }
    }
  ],
  "right_widgets": [
    {
      "Media": {
        "enable": true
      }
    },
    {
      "Storage": {
        "enable": true
      }
    },
    {
      "Memory": {
        "enable": true
      }
    },
    {
      "Date": {
        "enable": true,
        "format": "DayDateMonthYear"
      }
    },
    {
      "Time": {
        "enable": true,
        "format": "TwentyFourHour"
      }
    },
    {
      "Battery": {
        "enable": true
      }
    }
  ]
}

Hotkey Configuration

#Requires AutoHotkey v2.0.2
#SingleInstance Force

Komorebic(cmd) {
RunWait(format("komorebic.exe {}", cmd), , "Hide")
}

!q::Komorebic("close")
!m::Komorebic("minimize")

; Focus windows
!h::Komorebic("focus left")
!j::Komorebic("focus down")
!k::Komorebic("focus up")
!l::Komorebic("focus right")

!+[::Komorebic("cycle-focus previous")
!+]::Komorebic("cycle-focus next")

; Move windows
!+h::Komorebic("move left")
!+j::Komorebic("move down")
!+k::Komorebic("move up")
!+l::Komorebic("move right")

; Stack windows
#Left::Komorebic("stack left")
#Down::Komorebic("stack down")
#Up::Komorebic("stack up")
#Right::Komorebic("stack right")
!#Down::Komorebic("unstack")
#,::Komorebic("cycle-stack previous")
#.::Komorebic("cycle-stack next")

; Resize
#^Right::Komorebic("resize-axis horizontal increase")
#^Left::Komorebic("resize-axis horizontal decrease")
#^Up::Komorebic("resize-axis vertical increase")
#^Down::Komorebic("resize-axis vertical decrease")

; Manipulate windows
!t::Komorebic("toggle-float")
!f::Komorebic("toggle-monocle")

; Window manager options
!+r::Komorebic("retile")
!p::Komorebic("toggle-pause")

; Layouts
!x::Komorebic("flip-layout horizontal")
!y::Komorebic("flip-layout vertical")

; Workspaces
#1::Komorebic("focus-workspace 0")
#2::Komorebic("focus-workspace 1")
#3::Komorebic("focus-workspace 2")
#4::Komorebic("focus-workspace 3")
#5::Komorebic("focus-workspace 4")
#6::Komorebic("focus-workspace 5")
#7::Komorebic("focus-workspace 6")
#8::Komorebic("focus-workspace 7")

; Move windows across workspaces
#+1::Komorebic("move-to-workspace 0")
#+2::Komorebic("move-to-workspace 1")
#+3::Komorebic("move-to-workspace 2")
#+4::Komorebic("move-to-workspace 3")
#+5::Komorebic("move-to-workspace 4")
#+6::Komorebic("move-to-workspace 5")
#+7::Komorebic("move-to-workspace 6")
#+8::Komorebic("move-to-workspace 7")

; Run shortcuts
#t::Run("wezterm -e", , "Hide")

Output of komorebic check

No KOMOREBI_CONFIG_HOME detected, defaulting to C:\Users\THEkr

Looking for configuration files in C:\Users\THEkr

Found komorebi.json; this file can be passed to the start command with the --config flag

Found C:\Users\THEkr.config\whkdrc; key bindings will be loaded from here when whkd is started, and you can start it automatically using the --whkd flag

@davor-skontra davor-skontra added the bug Something isn't working label Jan 11, 2025
@davor-skontra davor-skontra changed the title [BUG]: Bar cannot display UWP Icons (PR with already available) [BUG]: Bar cannot display UWP Icons (PR with fix already exists) Jan 11, 2025
@LGUG2Z
Copy link
Owner

LGUG2Z commented Jan 11, 2025

Thanks for opening this PR! I actually didn't get notified of the underlying PR to the windows-icons repo, I'll take a look soon 🤞

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jan 12, 2025

I played with this a little today and the issue with UWP apps is that they are all running as ApplicationFrameworkHost.exe, and when you pass the process_id of these apps to the library to go and look up their icons, they can't be uniquely identified 🤔

Not sure how to proceed with this, maybe @amnweb has some ideas from yasb that we can bring here.

@amnweb
Copy link

amnweb commented Jan 12, 2025

UWP apps might need a moment to start under ApplicationFrameHost, maybe that's the problem?
An example of how I solved this

https://github.com/amnweb/yasb/blob/cd336fa718c39d30ac4eb1a8e29fd920fae5e38f/src/core/widgets/yasb/active_window.py#L195

@davor-skontra
Copy link
Author

Thanks for that Amnweb! I'll try the same approach!

@davor-skontra
Copy link
Author

Ok, I did some debugging and it looks like window-icons is actually able to get a path to the windows terminal icon (which exists in the file system, checked).

The process fails on the next step when trying to read image to base 64.

pub fn get_uwp_icon(process_path: &str) -> Result<RgbaImage, Box<dyn Error>> {
    let icon_path = &get_icon_file_path(process_path)?;
    let base64 = read_image_to_base64(icon_path)?;  // I never get beyond this point with Windows Terminal
    let icon = get_icon_from_base64(&base64)?;
    Ok(icon)
}

I think it has to do with the icon format. Arc Browser uses .ico, terminal uses .png . Arc Browser icon gets loaded, Terminal does not. Both are UWP apps.

I'm not sure how encodings work and what they do. also don't know much about these image formats in general.

I'll try to figure it out over the weekend but advice is welcome. 😁

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jan 15, 2025

I think I remember when I looked inside the path given for the file on my system, the file referenced didn't actually exist 👀 Can you check if this is the case for you also?

@davor-skontra
Copy link
Author

Double checked. Yeah you are right, that exact file does not exist.

window-icons is making use of the ApplicationMaifest.xml to figure out where the icon is, assuming it is within the <Logo> tags.

However Terminal seems to be doing something else with that.

The manifest has this

<Logo>Images\StoreLogo.png</Logo>

While the files have info appended to the name like here.

image

I'm not sure what's happening here or how common this approach is with other uwp applications.

@amnweb
Copy link

amnweb commented Jan 18, 2025

I think you need to find VisualElements in the manifest and look for Square44x44Logo, this should be a png, jpg or jpeg image and this is what you need for the icon.

Sorry I don't know about Rust, but here's what I mean, something like example below (Python) to cover all Win11 and Win10

root = ET.parse(manifest_path)
element = root.find(".//VisualElements")
if element is None:
    element = root.find(".//{http://schemas.microsoft.com/appx/manifest/uap/windows10}VisualElements")
if not element :
    return None
if "Square44x44Logo" not in element .attrib:
    return None

Then, once you get the Square44x44 logo path, you can easily manipulate the image as you wish.

For more complex function see this file https://github.com/amnweb/yasb/blob/main/src/core/utils/win32/app_icons.py

Edit:

Image

@davor-skontra
Copy link
Author

Thanks a bunch @amnweb !

Looks like there are a quite a few things that would need handling to get this working in a Microsoft way but a quick hack for now seems reasonable

Windows Terminal makes use of icon scaling so to solve I could just grab the closest Icon to 100% scale I can find and if that doesn't work I can search upward for a closest larger scale icon.

Another thing I didn't consider, but you seem to be handling to a degree are localizations, contrast and things like that. I don't have a clear plan there yet.

@davor-skontra
Copy link
Author

OK, got something working though could be more polished. More info in the original PR.

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jan 20, 2025

We are definitely getting closer! I can see the WT icon coming through properly now. The next issue to tackle will be handling the Icons for ApplicationFrameworkHost.exe apps like Settings, which are still showing what I imagine is a default/fallback icon from where:

Image

@davor-skontra
Copy link
Author

Alrriighty. That ApllicationFrameworkHost is another system then. I'll try to tackle that one as well.

Am also hoping to tackle scaling in such a way that you could pass that down as a param when calling get_icon_by_* methods

pub struct IconMatcher {
    pub display_scale: Option<i8>,
}
pub fn get_icon_by_process_id(process_id: u32, icon_matcher: Option<IconMatcher>) -> Option<RgbaImage> {
    let path = get_process_path(process_id).ok()?;

    if path.contains("WindowsApps") {
        get_uwp_icon(&path, icon_matcher).ok()
    } else {
        get_icon_by_path(&path)
    }
}

I think this is a better approach then the library trying to figure out what the user wants to do and reading the scale info directly from system.

If needed I can also do that in a non API breaking way by adding a separate method for using the matcher.

Side note, that IconMatcher struct would later also pass things like localization data down the line so we can get the correct icon for localized apps.

@LGUG2Z LGUG2Z added i-will-probably-work-on-this I'll work on this when I get time, but if someone beats me to it, thank you! komorebi-bar Related to the komorebi-bar crate labels Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working i-will-probably-work-on-this I'll work on this when I get time, but if someone beats me to it, thank you! komorebi-bar Related to the komorebi-bar crate
Projects
None yet
Development

No branches or pull requests

3 participants