jnsh / arc-theme

A flat theme with transparent elements (actively maintained fork)
GNU General Public License v3.0
902 stars 77 forks source link

gnome-shell: Add resources to compile Arc theme to GDM .gresource file #76

Closed drakkar1969 closed 3 years ago

drakkar1969 commented 4 years ago

Add default icon resources and *.gresource.xml files for compilation of Arc theme into GDM .gresource file.

To pick up on pending points from #72:

The icons in 1351e6f should be placed outside of the assets dir in common/gnome/shell//icons/. This way they can be only included in the gresource and not unnecessarily installed otherwise.

OK

I think you should be able to add the process-working.svg with rest of the icons, and fix the path with the alias= attribute, i.e. icons/process-working.svg.

Thanks, suggested solution works

I should be able to add automatic compilation and installation for the gresource in the build scripts quite easily. However that would basically require adding the correct *,gresource.xml files, and the additional icons for every shell version. This is a rather tedious task, but it would be amazing if you have time and interest to work on that while you're at it.

For now, only added icons and XML files for shell version 3.36. Once you are ok with file locations/naming, I can work on previous shell versions. The only SVG icon resource I have edited is color-pick.svg - i.e. the only non-symbolic icon, except for process-working.svg (on which there are issues already discussed).

An alternative to having "pre-made" *.gresource.xml files is to generate them on-the-fly. Sample script:

SOURCEDIR="<dir1>/gnome-shell"
XMLFILE="<dir2>/$(basename $1).xml"

FILELIST=$(find $SOURCEDIR -type f -printf "%P\n" | sort | xargs -i echo "    <file>{}</file>")

cat > $XMLFILE <<-RESXML
    <?xml version="1.0" encoding="UTF-8"?>
    <gresources>
      <gresource prefix="/org/gnome/shell/theme">
    $FILELIST
      </gresource>
    </gresources>
RESXML
jnsh commented 4 years ago

The only SVG icon resource I have edited is color-pick.svg

I don't think the icon should be edited. Looks like the idea is that the color picker icon previews the selected color [1], and I think the recoloring breaks this feature. Either way, the icon was only added for 3.38, so it would be better to not include it for the 3.36 theme.

For -dark and -lighter you should use <file alias="gnome-shell.css">gnome-shell-dark.css</file> and <file alias="gnome-shell.css">gnome-shell-lighter.css</file> instead of <file>gnome-shell.css</file>.

There are still some files present in the default gresource.xml that aren't included here yet. Some are certainly not necessary for Arc (gnome-shell-high-contrast.css, no-events.svg and no-notifications.svg) but are still called in the JavaScript, and will probably output an error (for some reason journalctl is broken in my test VM so I couldn't check this). As long as their absence doesn't result in massive error flood, or cause the Shell to crash, I think it's fine to not include them.

The pad-osd.css however is necessary at least for wacom tablet configuration dialog. I should still investigate whether it could be integrated in the _common.scss, or must be included in separate file (this has been in my TODO for some time), but I suppose for now we could just include the upstream file.

Otherwise everything seems OK so far.

drakkar1969 commented 4 years ago

The only SVG icon resource I have edited is color-pick.svg

I don't think the icon should be edited. Looks like the idea is that the color picker icon previews the selected color [1], and I think the recoloring breaks this feature. Either way, the icon was only added for 3.38, so it would be better to not include it for the 3.36 theme.

Removed

For -dark and -lighter you should use <file alias="gnome-shell.css">gnome-shell-dark.css</file> and <file alias="gnome-shell.css">gnome-shell-lighter.css</file> instead of <file>gnome-shell.css</file>.

Done

There are still some files present in the default gresource.xml that aren't included here yet. Some are certainly not necessary for Arc (gnome-shell-high-contrast.css, no-events.svg and no-notifications.svg) but are still called in the JavaScript, and will probably output an error (for some reason journalctl is broken in my test VM so I couldn't check this). As long as their absence doesn't result in massive error flood, or cause the Shell to crash, I think it's fine to not include them.

Not seeing any error in journalctl, and definitely no shell crashes

The pad-osd.css however is necessary at least for wacom tablet configuration dialog. I should still investigate whether it could be integrated in the _common.scss, or must be included in separate file (this has been in my TODO for some time), but I suppose for now we could just include the upstream file.

Added

jnsh commented 4 years ago

For -dark and -lighter you should use <file alias="gnome-shell.css">gnome-shell-dark.css</file> and <file alias="gnome-shell.css">gnome-shell-lighter.css</file> instead of <file>gnome-shell.css</file>.

Done

You missed the alias="gnome-shell.css" parts, the shell or GDM won't start without it. Apart from that everything seems to be in order.

If you're still planning on implementing this for older shell versions, make sure you also include assets that have been removed from newer versions, and also check the upstream gnome-shell-theme.gresource.xml in the respective upstream branches for each version.

I've also considered dropping some of the older gnome-shell theme versions from this repo, since AFAIK they aren't used for packaging for LTS distributions, and most people who compile by hand are probably using newer versions as well. I'm thinking of doing this shortly after the next version is released, which should be done once any GNOME 3.38 fixes and updates are implemented.

I still haven't figured out what the oldest supported version should be, but I don't think there's any point adding the gresource files to versions older than 3.28. It's fine by me if you want to wait for this decision until working on this any further.

Also, I mentioned earlier that I was considering adding the gresource compilation to the build, but I think that will be postponed for now. Since the only use for it is overriding the default theme, which has a chance of rendering systems unbootable if you don't know what you're doing, I really wouldn't want to have it compiled by default and added to distribution packages. This means there would have to be a build option to enable the gresource compilation, but I don't want to add options that only target single detail on a single theme until the build system is rewritten some time in the future. With the added *.gresource.xml files it is quite simple to compile manually anyways.

drakkar1969 commented 4 years ago

Also, I mentioned earlier that I was considering adding the gresource compilation to the build, but I think that will be postponed for now. Since the only use for it is overriding the default theme, which has a chance of rendering systems unbootable if you don't know what you're doing, I really wouldn't want to have it compiled by default and added to distribution packages. This means there would have to be a build option to enable the gresource compilation, but I don't want to add options that only target single detail on a single theme until the build system is rewritten some time in the future. With the added *.gresource.xml files it is quite simple to compile manually anyways.

I do think this may be a safer option, so that only those who know what they are doing compile into GDM using the *.gresource.xml files. However, if this is the case, the *.gresource.xml files - and the additional SVG/CSS resources - should probably be copied over into the installed theme folders. Not so much for those building from source, but mostly for people installing with distro packages - to avoid having to hunt down the source files for .gresource compilation. Also, the *.gresource.xml files would need to be corrected (e.g. include gnome-shell.css for all variants)

If you're still planning on implementing this for older shell versions, make sure you also include assets that have been removed from newer versions, and also check the upstream gnome-shell-theme.gresource.xml in the respective upstream branches for each version.

Been looking at that, was just trying to find a way to automate the process of creating the xml files as far as possible

jnsh commented 3 years ago

However, if this is the case, the *.gresource.xml files - and the additional SVG/CSS resources - should probably be copied over into the installed theme folders. Not so much for those building from source, but mostly for people installing with distro packages - to avoid having to hunt down the source files for .gresource compilation. Also, the *.gresource.xml files would need to be corrected (e.g. include gnome-shell.css for all variants)

It would be nice to have the gresouce-related files easily available, but I think including otherwise unnecessary source files to the default installation is kind of ugly, especially with all the necessary extra effort to the build system and gresource modifications. Also I'd imagine most users who'd replace the default theme gresource would also think of looking for the gresource source files from the project's code repository. I should also set up a FAQ for a few common issues, and the gresource situation could also be explained there.

If you're still planning on implementing this for older shell versions, make sure you also include assets that have been removed from newer versions, and also check the upstream gnome-shell-theme.gresource.xml in the respective upstream branches for each version.

Been looking at that, was just trying to find a way to automate the process of creating the xml files as far as possible

I think this has to be done mostly by hand. This is what I would do:

  1. Copy all files over to previous version
  2. Check for any additions/removals in the theme's *-assets/ directories compared to the previous version, and update the .gresource.xml files accordingly. At least the checking could probably be automated with some simple shell script.
  3. Check for changes in the upstream gnome-shell-theme.gresource.xml compared to the previous version (e.g. gnome-3-36 branch vs. gnome-3-34 and so on, this could probably be done with git diff somehow). Change the .gresource.xml files accordingly, and copy over or remove icons or other resource files if necessary.
  4. Repeat for next version until you're done

Either way, it's probably best to wait until the old-version-purging situation is clear, if that's fine with you

jnsh commented 3 years ago

I've also considered dropping some of the older gnome-shell theme versions from this repo

After some more thought, I think the old-version-purge will be postponed for now. I was hoping we could save some work with the gresource files by doing this now, but determining the oldest version that should be supported isn't that simple, and I don't want to do it hastily and risk making some users unhappy.

Therefore the gresources files should be added to all versions starting from 3.18. Let me know if you need any help with that.

As a side note, I've started working on adding a meson based build system. Once that's in good shape, the old build system can be deprecated, and I can start adding new features (such as the automatic gresource compilation) to the meson build.

jnsh commented 3 years ago

@drakkar1969 I noticed you had pushed new commits so I had a quick look, and one thing still requires fixing in the *.gresource.xml files: For Dark and Lighter variants, you must add alias="gnome-shell.css" when importing the CSS.

For example, for every gnome-shell-theme-dark.gresource.xml, you need to change <file>gnome-shell-dark.css</file> to <file alias="gnome-shell.css">gnome-shell-dark.css</file>.

Without this gdm and gnome-shell won't start, since they are looking for gnome-shell.css.

I mentioned this earlier, but I may have been a bit unclear, or maybe you just missed this within the wall of text comment.

drakkar1969 commented 3 years ago

@drakkar1969 I noticed you had pushed new commits so I had a quick look, and one thing still requires fixing in the *.gresource.xml files: For Dark and Lighter variants, you must add alias="gnome-shell.css" when importing the CSS.

Done now, comment had slipped through the cracks.

Also wanted to check a few things with you:

1. GNOME shell 3.34/3.32/3.30: Some resources are listed in the default XML file with a icons/* alias, e.g.:

<file alias="icons/message-indicator-symbolic.svg">message-indicator-symbolic.svg</file>

When these resources are added to the Arc theme, they are already in an icons sub-folder, so I have removed the alias in the Arc XML files, e.g.:

<file>icons/message-indicator-symbolic.svg</file>

The alternative would have been:

<file alias="icons/message-indicator-symbolic.svg">icons/message-indicator-symbolic.svg</file>

Which appears to be redundant

2. GNOME shell 3.32/3.30:

The process-working.svg resource is currently present in both common-assets/misc and icons sub-folders - and appears twice in the Arc XML files:

<file>common-assets/misc/process-working.svg</file>
<file alias="process-working.svg">icons/process-working.svg</file>

I believe the two lines above can be replaced with a single line in the XML files (and the process-working.svg resource in the icons sub-folder removed):

<file alias="process-working.svg">common-assets/misc/process-working.svg</file>

3. GNOME shell 3.32:

There are a few Arc-specific assets missing in the Arc-Lighter variant, compared to the other variants:

lighter-assets/menu/menu.svg
lighter-assets/menu/submenu-open.svg
lighter-assets/menu/submenu.svg
lighter-assets/misc/message-active.svg
lighter-assets/misc/message-close-active.svg
lighter-assets/misc/message-close-hover.svg
lighter-assets/misc/message-close.svg
lighter-assets/misc/message-hover.svg
lighter-assets/misc/message.svg
lighter-assets/misc/modal.svg

It appears they were intentionally not added to the newly-introduced Lighter variant because unnecessary/obsolete, but just wanted to check in case I am missing something

jnsh commented 3 years ago

Also wanted to check a few things with you:

1. GNOME shell 3.34/3.32/3.30: Some resources are listed in the default XML file with a icons/* alias, e.g.:

<file alias="icons/message-indicator-symbolic.svg">message-indicator-symbolic.svg</file>

When these resources are added to the Arc theme, they are already in an icons sub-folder, so I have removed the alias in the Arc XML files, e.g.:

<file>icons/message-indicator-symbolic.svg</file>

The alternative would have been:

<file alias="icons/message-indicator-symbolic.svg">icons/message-indicator-symbolic.svg</file>

Which appears to be redundant

Correct, the alias= option is only necessary if the asset path or file name differs from what gnome-shell expects, i.e. the path and file name (or the value from alias= if present) from the upstream gnome-shell-theme.gresource.xml. So, in these cases the aliases aren't necessary.

2. GNOME shell 3.32/3.30:

The process-working.svg resource is currently present in both common-assets/misc and icons sub-folders - and appears twice in the Arc XML files:

<file>common-assets/misc/process-working.svg</file>
<file alias="process-working.svg">icons/process-working.svg</file>

I believe the two lines above can be replaced with a single line in the XML files (and the process-working.svg resource in the icons sub-folder removed):

<file alias="process-working.svg">common-assets/misc/process-working.svg</file>

I'm not sure about this without testing, although from the gresource documentation it sounds like the alias would override the original path and filename, meaning you'd need the two separate entries, although the wording is a bit ambiguous:

The alias attribute can be used to alter the filename to expose them at a different location in the resource namespace.

Since this only affects older versions, I think you could just add the process-working.svg asset twice. The only drawback is that it increases the gresource size slightly. I think it's better to just play safe here, and not spend extra time investigating this.

3. GNOME shell 3.32:

There are a few Arc-specific assets missing in the Arc-Lighter variant, compared to the other variants:

lighter-assets/menu/menu.svg
lighter-assets/menu/submenu-open.svg
lighter-assets/menu/submenu.svg
lighter-assets/misc/message-active.svg
lighter-assets/misc/message-close-active.svg
lighter-assets/misc/message-close-hover.svg
lighter-assets/misc/message-close.svg
lighter-assets/misc/message-hover.svg
lighter-assets/misc/message.svg
lighter-assets/misc/modal.svg

It appears they were intentionally not added to the newly-introduced Lighter variant because unnecessary/obsolete, but just wanted to check in case I am missing something

In 3.32 theme, some of the assets were replaced with CSS, which in turn allowed adding the lighter variant. Therefore the assets that aren't present for lighter variant, aren't necessary and can be excluded from the gresource for all variants.

drakkar1969 commented 3 years ago

Since this only affects older versions, I think you could just add the process-working.svg asset twice. The only drawback is that it increases the gresource size slightly. I think it's better to just play safe here, and not spend extra time investigating this.

Ok, will go with the safe option, i.e. including the asset twice

In 3.32 theme, some of the assets were replaced with CSS, which in turn allowed adding the lighter variant. Therefore the assets that aren't present for lighter variant, aren't necessary and can be excluded from the gresource for all variants.

3.32 XML files updated (for Light/Dark variants) to exclude unnecessary assets

drakkar1969 commented 3 years ago

Extras resources and XML files for GDM compilation should now be ok for all GNOME shell versions

jnsh commented 3 years ago

Excellent! Thank you for the huge effort!

One thing I hope I had noticed earlier, is the location for the gresource.xml source files. My idea was to add these in the base common/gnome-shell/<version>/ directory, but having them in a subdirectory is certainly a bit cleaner, and apparently works just as well, as long as the resource compilation is performed from the base theme directory.

However I think the subdirectory would be better named just gresource/ instead of gresource-xml/ since the compiled .gresource files end up in the same location.

I can fixup this quite easily when merging, as I'd like to squash the commits into one anyway, if that's fine with you?

Apart from that I think everything should be in order, but I'd prefer to merge this only after I get the next release tarball rolled out, hopefully in a few days.

drakkar1969 commented 3 years ago

However I think the subdirectory would be better named just gresource/ instead of gresource-xml/ since the compiled .gresource files end up in the same location.

I can fixup this quite easily when merging, as I'd like to squash the commits into one anyway, if that's fine with you?

Makes perfect sense - thought about squashing the commits myself, but it was just easier for me to keep the commits separate as I went through the GNOME versions :)

jnsh commented 3 years ago

Merged squashed into https://github.com/jnsh/arc-theme/commit/04de6c7862a37126182a8133a148bc647383be6f, and with the gresource-xml/ directory renamed as detailed above.