Skip to content

Background: Use GLib.File instead of dealing with Paths #1711

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.

Sign up for GitHub

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

Open
tintou wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Background: Use GLib.File instead of dealing with Paths #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c0bcbb
Background: Use GLib.File instead of dealing with Paths
tintou Jul 4, 2023
186d4f4
Merge branch 'master' into tintou/background-no-path
lenemter Jul 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions src/Background/Animation.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,20 @@

namespace Gala {
public class Animation : Object {
public string filename { get; construct; }
public string[] key_frame_files { get; private set; default = {}; }
public GLib.File file { get; construct; }
public GLib.GenericArray<GLib.File> key_frame_files { get; private set; default = new GLib.GenericArray<GLib.File> (); }
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

Does this need to be a GenericArray? given that we known that the max number of files we will use is 2, isn't a fixed array enough.

public double transition_progress { get; private set; default = 0.0; }
public double transition_duration { get; private set; default = 0.0; }
public bool loaded { get; private set; default = false; }

private Gnome.BGSlideShow? show = null;

public Animation (string filename) {
Object (filename: filename);
public Animation (GLib.File file) {
Object (file: file);
}

public async void load () {
show = new Gnome.BGSlideShow (filename);
show = new Gnome.BGSlideShow (file.get_path ());

show.load_async (null, (obj, res) => {
loaded = true;
Expand All @@ -42,7 +42,7 @@ namespace Gala {
}

public void update (Meta.Rectangle monitor) {
string[] key_frame_files = {};
var new_key_frame_files = new GLib.GenericArray<GLib.File> ();

if (show == null)
return;
Expand All @@ -52,19 +52,21 @@ namespace Gala {

double progress, duration;
bool is_fixed;
string file1, file2;
unowned string? file1, file2;
show.get_current_slide (monitor.width, monitor.height, out progress, out duration, out is_fixed, out file1, out file2);

transition_duration = duration;
transition_progress = progress;

if (file1 != null)
key_frame_files += file1;
if (file1 != null) {
new_key_frame_files.add (GLib.File.new_for_path (file1));
}

if (file2 != null)
key_frame_files += file2;
if (file2 != null) {
new_key_frame_files.add (GLib.File.new_for_path (file2));
}

this.key_frame_files = key_frame_files;
this.key_frame_files = new_key_frame_files;
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

i don't think the this accessor is needed anymore here.

}
}
}
70 changes: 36 additions & 34 deletions src/Background/Background.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,39 +28,39 @@ namespace Gala {
public BackgroundSource background_source { get; construct; }
public bool is_loaded { get; private set; default = false; }
public GDesktop.BackgroundStyle style { get; construct; }
public string? filename { get; construct; }
public GLib.File? file { get; construct; }
public Meta.Background background { get; private set; }

private Animation? animation = null;
private Gee.HashMap<string,ulong> file_watches;
private Gee.HashMap<GLib.File, ulong> file_watches;
private Cancellable cancellable;
private uint update_animation_timeout_id = 0;

private Gnome.WallClock clock;
private ulong clock_timezone_handler = 0;

public Background (Meta.Display display, int monitor_index, string? filename,
public Background (Meta.Display display, int monitor_index, GLib.File? file,
BackgroundSource background_source, GDesktop.BackgroundStyle style) {
Object (display: display,
monitor_index: monitor_index,
background_source: background_source,
style: style,
filename: filename);
file: file);
}

construct {
background = new Meta.Background (display);
background.set_data<unowned Background> ("delegate", this);

file_watches = new Gee.HashMap<string,ulong> ();
file_watches = new Gee.HashMap<GLib.File, ulong> ();
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

Use GLib.File.hash and GLib.File.equal here? Gee will fallback to direct_{hash,equal} if the functions aren't provided here.

cancellable = new Cancellable ();

background_source.changed.connect (settings_changed);

clock = new Gnome.WallClock ();
clock_timezone_handler = clock.notify["timezone"].connect (() => {
if (animation != null) {
load_animation.begin (animation.filename);
load_animation.begin (animation.file);
}
});

Expand Down Expand Up @@ -121,18 +121,18 @@ namespace Gala {
}
}

private void watch_file (string filename) {
if (file_watches.has_key (filename))
private void watch_file (GLib.File file) {
if (file_watches.has_key (file))
return;
Comment on lines +125 to 126
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

Code style.

Suggested change
if (file_watches.has_key (file))
return;
if (file_watches.has_key (file)) {
return;
}


var cache = BackgroundCache.get_default ();

cache.monitor_file (filename);
cache.monitor_file (file);

file_watches[filename] = cache.file_changed.connect ((changed_file) => {
if (changed_file == filename) {
file_watches[file] = cache.file_changed.connect ((changed_file) => {
if (changed_file == file) {
var image_cache = Meta.BackgroundImageCache.get_default ();
image_cache.purge (File.new_for_path (changed_file));
image_cache.purge (changed_file);
changed ();
}
});
Expand All @@ -145,13 +145,13 @@ namespace Gala {
}
}

private void finish_animation (string[] files) {
private void finish_animation (GLib.GenericArray<GLib.File> files) {
set_loaded ();

if (files.length > 1)
background.set_blend (File.new_for_path (files[0]), File.new_for_path (files[1]), animation.transition_progress, style);
background.set_blend (files[0], files[1], animation.transition_progress, style);
else if (files.length > 0)
background.set_file (File.new_for_path (files[0]), style);
background.set_file (files[0], style);
else
background.set_file (null, style);

Expand All @@ -164,12 +164,12 @@ namespace Gala {
animation.update (display.get_monitor_geometry (monitor_index));
var files = animation.key_frame_files;

var cache = Meta.BackgroundImageCache.get_default ();
unowned var cache = Meta.BackgroundImageCache.get_default ();
var num_pending_images = files.length;
for (var i = 0; i < files.length; i++) {
watch_file (files[i]);
foreach (unowned var file in files) {
watch_file (file);

var image = cache.load (File.new_for_path (files[i]));
var image = cache.load (file);

if (image.is_loaded ()) {
num_pending_images--;
Expand Down Expand Up @@ -213,24 +213,24 @@ namespace Gala {
});
}

private async void load_animation (string filename) {
animation = yield BackgroundCache.get_default ().get_animation (filename);
private async void load_animation (GLib.File file) {
animation = yield BackgroundCache.get_default ().get_animation (file);

if (animation == null || cancellable.is_cancelled ()) {
set_loaded ();
return;
}

update_animation ();
watch_file (filename);
watch_file (file);
}

private void load_image (string filename) {
background.set_file (File.new_for_path (filename), style);
watch_file (filename);
private void load_image (GLib.File file) {
background.set_file (file, style);
watch_file (file);

var cache = Meta.BackgroundImageCache.get_default ();
var image = cache.load (File.new_for_path (filename));
var image = cache.load (file);
if (image.is_loaded ())
set_loaded ();
else {
Expand All @@ -242,20 +242,22 @@ namespace Gala {
}
}

private void load_file (string filename) {
if (filename.has_suffix (".xml"))
load_animation.begin (filename);
else
load_image (filename);
private inline void load_file (GLib.File file) {
if (file.get_basename ().has_suffix (".xml")) {
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

i think we should use GLib.ContentType.guess() here, so that we can identify xml files without a extension. like the ones that the Gtk portal provides.

load_animation.begin (file);
} else {
load_image (file);
}
}

private void load () {
load_pattern ();

if (filename == null)
if (file == null) {
set_loaded ();
else
load_file (filename);
} else {
load_file (file);
}
}

private void settings_changed () {
Expand Down
23 changes: 11 additions & 12 deletions src/Background/BackgroundCache.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ namespace Gala {
return instance;
}

public signal void file_changed (string filename);
public signal void file_changed (GLib.File file);

private Gee.HashMap<string,FileMonitor> file_monitors;
private Gee.HashMap<GLib.File,FileMonitor> file_monitors;
private Gee.HashMap<string,BackgroundSource> background_sources;

private Animation animation;
Expand All @@ -38,29 +38,28 @@ namespace Gala {
}

construct {
file_monitors = new Gee.HashMap<string,FileMonitor> ();
file_monitors = new Gee.HashMap<GLib.File, FileMonitor> ((Gee.HashDataFunc) GLib.File.hash, (Gee.EqualDataFunc) GLib.File.equal);
background_sources = new Gee.HashMap<string,BackgroundSource> ();
}

public void monitor_file (string filename) {
if (file_monitors.has_key (filename))
public void monitor_file (GLib.File file) {
if (file_monitors.has_key (file))
return;

var file = File.new_for_path (filename);
try {
var monitor = file.monitor (FileMonitorFlags.NONE, null);
monitor.changed.connect (() => {
file_changed (filename);
file_changed (file);
});

file_monitors[filename] = monitor;
file_monitors[file] = monitor;
} catch (Error e) {
warning ("Failed to monitor %s: %s", filename, e.message);
warning ("Failed to monitor %s: %s", file.get_path (), e.message);
}
}

public async Animation get_animation (string filename) {
if (animation != null && animation.filename == filename) {
public async Animation get_animation (GLib.File file) {
if (animation != null && animation.file == file) {
Idle.add (() => {
get_animation.callback ();
return false;
Expand All @@ -70,7 +69,7 @@ namespace Gala {
return animation;
}

var animation = new Animation (filename);
var animation = new Animation (file);

yield animation.load ();

Expand Down
10 changes: 5 additions & 5 deletions src/Background/BackgroundSource.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,26 @@ namespace Gala {
}

public Background get_background (int monitor_index) {
string? filename = null;
GLib.File? file = null;

var style = settings.get_enum ("picture-options");
if (style != GDesktop.BackgroundStyle.NONE) {
var uri = settings.get_string ("picture-uri");
if (Uri.parse_scheme (uri) != null)
filename = File.new_for_uri (uri).get_path ();
file = File.new_for_uri (uri);
else
filename = uri;
file = File.new_for_path (uri);
}

// Animated backgrounds are (potentially) per-monitor, since
// they can have variants that depend on the aspect ratio and
// size of the monitor; for other backgrounds we can use the
// same background object for all monitors.
if (filename == null || !filename.has_suffix (".xml"))
if (file == null || !file.get_basename ().has_suffix (".xml"))
Copy link
Contributor

@Marukesu Marukesu Jul 6, 2023

Choose a reason for hiding this comment

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

Same thing here, should check the mimetype instead of extension.

monitor_index = 0;

if (!backgrounds.has_key (monitor_index)) {
var background = new Background (display, monitor_index, filename, this, (GDesktop.BackgroundStyle) style);
var background = new Background (display, monitor_index, file, this, (GDesktop.BackgroundStyle) style);
background.changed.connect (background_changed);
backgrounds[monitor_index] = background;
}
Expand Down
2 changes: 1 addition & 1 deletion vapi/gnome-desktop-3.0.vapi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ namespace Gnome {
public class BGSlideShow : GLib.Object {
[CCode (has_construct_function = false)]
public BGSlideShow (string filename);
public void get_current_slide (int width, int height, out double progress, out double duration, out bool is_fixed, out unowned string file1, out unowned string file2);
public void get_current_slide (int width, int height, out double progress, out double duration, out bool is_fixed, out unowned string? file1, out unowned string? file2);
public bool get_has_multiple_sizes ();
public int get_num_slides ();
public bool get_slide (int frame_number, int width, int height, out double progress, out double duration, out bool is_fixed, out unowned string file1, out unowned string file2);
Expand Down