From d839c73e14272c6ec246336e37d05ccb6ef17c78 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 11 May 2022 13:40:23 -0500 Subject: [PATCH 01/44] New CairoContextState struct to capture the state of a passed-in Cairo context The idea is to log all "transactions" in the public API, including the state of the Cairo context that was passed to those calls. Part-of: --- src/log.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/log.rs b/src/log.rs index 346fe25e7..505ae0862 100644 --- a/src/log.rs +++ b/src/log.rs @@ -18,3 +18,53 @@ pub fn log_enabled() -> bool { *ENABLED } + +/// Captures the basic state of a [`cairo::Context`] for logging purposes. +/// +/// A librsvg "transaction" like rendering a +/// [`crate::api::SvgHandle`], which takes a Cairo context, depends on the state of the +/// context as it was passed in by the caller. For example, librsvg may decide to +/// operate differently depending on the context's target surface type, or its current +/// transformation matrix. This struct captures that sort of information. +#[derive(Copy, Clone, Debug, PartialEq)] +struct CairoContextState { + surface_type: cairo::SurfaceType, +} + +impl CairoContextState { + fn new(cr: &cairo::Context) -> Self { + let surface_type = cr.target().type_(); + + Self { surface_type } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn captures_cr_surface_type() { + let surface = cairo::ImageSurface::create(cairo::Format::ARgb32, 10, 10).unwrap(); + let cr = cairo::Context::new(&surface).unwrap(); + let state = CairoContextState::new(&cr); + + assert_eq!( + CairoContextState { + surface_type: cairo::SurfaceType::Image, + }, + state, + ); + + let surface = cairo::RecordingSurface::create(cairo::Content::ColorAlpha, None).unwrap(); + let cr = cairo::Context::new(&surface).unwrap(); + let state = CairoContextState::new(&cr); + + assert_eq!( + CairoContextState { + surface_type: cairo::SurfaceType::Recording, + }, + state, + ); + } +} -- GitLab From 83e2fc90ffe9e74cca34238f333e99f2ab8aeb12 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 11 May 2022 13:55:25 -0500 Subject: [PATCH 02/44] devel-docs/api-observability.md: Development docs for observability Part-of: --- devel-docs/api-observability.md | 131 ++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 devel-docs/api-observability.md diff --git a/devel-docs/api-observability.md b/devel-docs/api-observability.md new file mode 100644 index 000000000..654494f71 --- /dev/null +++ b/devel-docs/api-observability.md @@ -0,0 +1,131 @@ +# Observability + +Librsvg supports basic, mostly ad-hoc logging with an `RSVG_LOG=1` +environment variable. This has not been very effective in letting me, +the maintainer, know what went wrong when someone reports a bug about +librsvg doing the wrong thing in an application. Part of it is +because the code could be more thorough about logging (e.g. log at all +error points), but also part of it is that there is no logging about +what API calls are made into the library. On each bug reported on +behalf of a particular application, my thought process goes something +like this: + +* What was the app doing? + +* Can I obtain the problematic SVG? + +* Does the bug reporter even know what the problematic SVG was? + +* Was the app rendering with direct calls to the librsvg API? + +* Or was it using the gdk-pixbuf loader, and thus has very little control of how librsvg is used? + +* If non-pixbuf, what was the Cairo state when librsvg was called? + +* What sort of interesting API calls are being used? Stylesheet + injection? Re-rendering single elements with different styles? + +And every time, I must ask the bug reporter for information related to +that, or to point me to the relevant source code where they were using +librsvg... which is not terribly useful, since building their code and +reproducing the bug with it is A Yak That Should Not Have To Be +Shaved. + +## Desiderata + + +## Stuff to log + + + +Log cr state at entry. + +Log name/base_uri of rendered document. + +Can we know if it is a gresource? Or a byte buffer? Did it come from gdk-pixbuf? + +## Invocation + +RSVG_LOG=1 is easy for specific processes or rsvg-convert + +Login processes like gnome-shell need a config file. ~/.config/librsvg.toml: + + [logging] + enabled=true # make this the default if the file exists? + process=gnome-shell # mandatory - don't want to log all processes - warn to g_log if key is not set + output=/home/federico/rsvg.log # if missing, log to g_log only + +/home/federico/rsvg.log - json doesn't have comments; put this in a string somehow: + ****************************************************************************** + * This log file exists because you enabled logging in ~/.config/librsvg.toml * + * for the "gnome-shell" process. * + * * + * If you want to disable this kind of log, please turn it off in that file * + * or delete that file entirely. * + ****************************************************************************** + + ****************************************************************************** + * This log file exists because you enabled logging with * + * RSVG_LOG_CONFIG=config.toml for the "single-process-name" process. * + * * + * If you want to disable this kind of log, FIXME */ + ****************************************************************************** + +** To-do list [0/1] + +- [ ] Audit code for GIO errors; log there. + +- [ ] Audit code for Cairo calls that yield errors; log there. + +- [ ] Log the entire ancestry of the element that caused the error? Is that an insta-reproducer? + +** Ideas + +*** Log API calls? + +Is this useful? Not all the entry points; most cases are new_from_whatever() / render(). + +Better, log the filename, or the base_uri for a stream, or optionally exfiltrate the SVG in case of a resource or raw data. + +*** What to log + +Entry point at rendering: state of the Cairo context, surface type, starting transform, etc. + +Versions of dependencies - pango, cairo, etc. Distro name / Windows / MacOS? + +*** Limit to a process + +For global configuration (see below), put the process name in the configuration file. + +For single-process config, use RSVG_LOG_CONFIG=filename.toml env var + + +*** Configuration and log format + +~/.config/librsvg.toml - global configuration + [logging] + enabled=true + process=gnome-shell # mandatory - don't want to log all processes + output=/home/federico/rsvg.log + +/home/federico/rsvg.log - json doesn't have comments; put this in a string somehow: + ****************************************************************************** + * This log file exists because you enabled logging in ~/.config/librsvg.toml * + * for the "gnome-shell" process. * + * * + * If you want to disable this kind of log, please turn it off in that file * + * or delete that file entirely. * + ****************************************************************************** + + ****************************************************************************** + * This log file exists because you enabled logging with * + * RSVG_LOG_CONFIG=config.toml for the "single-process-name" process. * + * * + * If you want to disable this kind of log, FIXME */ + ****************************************************************************** + +Output JSON, so it can nest and such? + +Add a replayer? This would effectively be "paint the render tree". +Just replay the user's provided log file, reproduce the bug. + -- GitLab From fe81d49621a5b80c8ee4d74203218aaec6da1738 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 11 May 2022 16:01:12 -0500 Subject: [PATCH 03/44] Desiderata for observability Part-of: --- devel-docs/api-observability.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/devel-docs/api-observability.md b/devel-docs/api-observability.md index 654494f71..2bb5a923c 100644 --- a/devel-docs/api-observability.md +++ b/devel-docs/api-observability.md @@ -33,6 +33,23 @@ Shaved. ## Desiderata +Know exactly what an application did with librsvg: + +* All API calls and their parameters. + +* State of the Cairo context at entry. + +* "What SVG?" - be careful and explicit about exfiltrating SVG data to the logs. + +Internals of the library: + +* Regular debug tracing. We may have options to enable/disable + tracing domains: parsing, cascading, referencing elements, temporary + surfaces during filtering, render tree, etc. + +* Log all points where an error is detected/generated, even if it will + be discarded later (e.g. invalid CSS values are silently ignored, + per the spec). ## Stuff to log -- GitLab From 4a4351cfde1bc4b8af7937a2f8d776ec7a9a43f0 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 11 May 2022 17:54:19 -0500 Subject: [PATCH 04/44] More details on the design doc for observability Part-of: --- devel-docs/api-observability.md | 137 ++++++++++++++++++++------------ 1 file changed, 87 insertions(+), 50 deletions(-) diff --git a/devel-docs/api-observability.md b/devel-docs/api-observability.md index 2bb5a923c..cd0ca91a4 100644 --- a/devel-docs/api-observability.md +++ b/devel-docs/api-observability.md @@ -41,6 +41,12 @@ Know exactly what an application did with librsvg: * "What SVG?" - be careful and explicit about exfiltrating SVG data to the logs. +* Basic platform stuff? Is the platform triple enough? Distro ID? + +* Versions of dependencies. + +* Version of librsvg itself. + Internals of the library: * Regular debug tracing. We may have options to enable/disable @@ -50,82 +56,110 @@ Internals of the library: * Log all points where an error is detected/generated, even if it will be discarded later (e.g. invalid CSS values are silently ignored, per the spec). + +## Enabling logging -## Stuff to log +It may be useful to be able to enable logging in various ways: +* Programmatically, for when one has control of the source code of the + problematic application. Enable logging at the problem spot, for + the SVG you know that exhibits the problem, and be done with it. + This can probably be at the individual `RsvgHandle` level, not + globally. For global logging within a single process, see the next + point. + +* For a single process which one can easily launch via the command + line; e.g. with an environment variable. This works well for + non-sandboxed applications. Something like + `RSVG_LOG_CONFIG=my_log_config.toml`. + +* With a configuration file, a la `~/.config/librsvg.toml`. Many + programs use librsvg and you don't want logs for all of them; allow + the configuration file to specify a process name, or maybe other + ways of determining when to log. For session programs like + gnome-shell, you can't easily set an environment variable to enable + logging - hence, a configuration file that only turns on logging + from the gnome-shell process. + +All of the above should be well documented, and then we can deprecate `RSVG_LOG`. +## Which SVG caused a crash? -Log cr state at entry. +Every once in a while, a bug report comes in like "$application +crashed in librsvg". The application renders many SVGs, often +indirectly via gdk-pixbuf, and it is hard to know exactly which SVG +caused the problem. Think of gnome-shell or gnome-software. -Log name/base_uri of rendered document. +For applications that call librsvg directly, if they pass the filename +or a GFile then it is not hard to find out the source SVG. -Can we know if it is a gresource? Or a byte buffer? Did it come from gdk-pixbuf? +But for those that feed bytes into librsvg, including those that use +it indirectly via gdk-pixbuf, librsvg has no knowledge of the +filename. We need to use the base_uri then, or see if the pixbuf +loader can be modified to propagate this information (is it even +available from the GdkPixbufLoader machinery?). -## Invocation +If all else fails, we can have an exfiltration mechanism. How can we +avoid logging *all* the SVG data that gnome-shell renders, for +example? Configure the logger to skip the first N SVGs, and hope that +the order is deterministic? We can't really "log only if there is a +crash during rendering". -RSVG_LOG=1 is easy for specific processes or rsvg-convert +Log only the checksums of SVGs or data lengths, and use that to find +which SVG caused the crash? I.e. have the user use a two-step process +to find a crash: get a log (written synchronously) of all SVG +checksums/lengths, and then reconfigure the logger to only exfiltrate +the last one that got logged - presumably that one caused the crash. -Login processes like gnome-shell need a config file. ~/.config/librsvg.toml: +## Which dynamically-created SVG caused a problem? - [logging] - enabled=true # make this the default if the file exists? - process=gnome-shell # mandatory - don't want to log all processes - warn to g_log if key is not set - output=/home/federico/rsvg.log # if missing, log to g_log only +Consider a bug like +https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5415 where an +application dynamically generates an SVG and feeds it to librsvg. +That bug was not a crash; it was about incorrect values returned from +an librsvg API function. For those cases it may be useful to be able +to exfiltrate an SVG and its stylesheets only if it matches a +user-provided substring. -/home/federico/rsvg.log - json doesn't have comments; put this in a string somehow: - ****************************************************************************** - * This log file exists because you enabled logging in ~/.config/librsvg.toml * - * for the "gnome-shell" process. * - * * - * If you want to disable this kind of log, please turn it off in that file * - * or delete that file entirely. * - ****************************************************************************** - - ****************************************************************************** - * This log file exists because you enabled logging with * - * RSVG_LOG_CONFIG=config.toml for the "single-process-name" process. * - * * - * If you want to disable this kind of log, FIXME */ - ****************************************************************************** +## Global configuration -** To-do list [0/1] +`$(XDG_CONFIG_HOME)/librsvg.toml` - for startup-like processes like +gnome-shell, for which it is hard to set an environment variable: -- [ ] Audit code for GIO errors; log there. +## Per-process configuration -- [ ] Audit code for Cairo calls that yield errors; log there. +`RSVG_LOG_CONFIG=my_log_config.toml my_process` -- [ ] Log the entire ancestry of the element that caused the error? Is that an insta-reproducer? -** Ideas -*** Log API calls? +## Programmatic API -Is this useful? Not all the entry points; most cases are new_from_whatever() / render(). +FIXME -Better, log the filename, or the base_uri for a stream, or optionally exfiltrate the SVG in case of a resource or raw data. -*** What to log +## Configuration format -Entry point at rendering: state of the Cairo context, surface type, starting transform, etc. +```toml +[logging] +enabled=true +process=gnome-shell # mandatory for global config - don't want to log all processes - warn to g_log if key is not set +output=/home/username/rsvg.log # if missing, log to g_log only - or use a output_to_g_log=true instead? +``` -Versions of dependencies - pango, cairo, etc. Distro name / Windows / MacOS? -*** Limit to a process +## API logging -For global configuration (see below), put the process name in the configuration file. +Log cr state at entry, surface type, starting transform. -For single-process config, use RSVG_LOG_CONFIG=filename.toml env var +Log name/base_uri of rendered document. +Can we know if it is a gresource? Or a byte buffer? Did it come from gdk-pixbuf? -*** Configuration and log format +## Log contents -~/.config/librsvg.toml - global configuration - [logging] - enabled=true - process=gnome-shell # mandatory - don't want to log all processes - output=/home/federico/rsvg.log +/home/username/rsvg.log - json doesn't have comments; put one of these in a string somehow: -/home/federico/rsvg.log - json doesn't have comments; put this in a string somehow: +``` ****************************************************************************** * This log file exists because you enabled logging in ~/.config/librsvg.toml * * for the "gnome-shell" process. * @@ -140,9 +174,12 @@ For single-process config, use RSVG_LOG_CONFIG=filename.toml env var * * * If you want to disable this kind of log, FIXME */ ****************************************************************************** +``` -Output JSON, so it can nest and such? +** To-do list [0/1] -Add a replayer? This would effectively be "paint the render tree". -Just replay the user's provided log file, reproduce the bug. +- [ ] Audit code for GIO errors; log there. +- [ ] Audit code for Cairo calls that yield errors; log there. + +- [ ] Log the entire ancestry of the element that caused the error? Is that an insta-reproducer? -- GitLab From 143b02fa460d126aba0b61de9a1663a60ae7d958 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 11:37:57 -0500 Subject: [PATCH 05/44] Convert the observability doc to rst Part-of: --- devel-docs/api-observability.md | 185 ----------------------------- devel-docs/api_observability.rst | 196 +++++++++++++++++++++++++++++++ devel-docs/index.rst | 4 +- 3 files changed, 198 insertions(+), 187 deletions(-) delete mode 100644 devel-docs/api-observability.md create mode 100644 devel-docs/api_observability.rst diff --git a/devel-docs/api-observability.md b/devel-docs/api-observability.md deleted file mode 100644 index cd0ca91a4..000000000 --- a/devel-docs/api-observability.md +++ /dev/null @@ -1,185 +0,0 @@ -# Observability - -Librsvg supports basic, mostly ad-hoc logging with an `RSVG_LOG=1` -environment variable. This has not been very effective in letting me, -the maintainer, know what went wrong when someone reports a bug about -librsvg doing the wrong thing in an application. Part of it is -because the code could be more thorough about logging (e.g. log at all -error points), but also part of it is that there is no logging about -what API calls are made into the library. On each bug reported on -behalf of a particular application, my thought process goes something -like this: - -* What was the app doing? - -* Can I obtain the problematic SVG? - -* Does the bug reporter even know what the problematic SVG was? - -* Was the app rendering with direct calls to the librsvg API? - -* Or was it using the gdk-pixbuf loader, and thus has very little control of how librsvg is used? - -* If non-pixbuf, what was the Cairo state when librsvg was called? - -* What sort of interesting API calls are being used? Stylesheet - injection? Re-rendering single elements with different styles? - -And every time, I must ask the bug reporter for information related to -that, or to point me to the relevant source code where they were using -librsvg... which is not terribly useful, since building their code and -reproducing the bug with it is A Yak That Should Not Have To Be -Shaved. - -## Desiderata - -Know exactly what an application did with librsvg: - -* All API calls and their parameters. - -* State of the Cairo context at entry. - -* "What SVG?" - be careful and explicit about exfiltrating SVG data to the logs. - -* Basic platform stuff? Is the platform triple enough? Distro ID? - -* Versions of dependencies. - -* Version of librsvg itself. - -Internals of the library: - -* Regular debug tracing. We may have options to enable/disable - tracing domains: parsing, cascading, referencing elements, temporary - surfaces during filtering, render tree, etc. - -* Log all points where an error is detected/generated, even if it will - be discarded later (e.g. invalid CSS values are silently ignored, - per the spec). - -## Enabling logging - -It may be useful to be able to enable logging in various ways: - -* Programmatically, for when one has control of the source code of the - problematic application. Enable logging at the problem spot, for - the SVG you know that exhibits the problem, and be done with it. - This can probably be at the individual `RsvgHandle` level, not - globally. For global logging within a single process, see the next - point. - -* For a single process which one can easily launch via the command - line; e.g. with an environment variable. This works well for - non-sandboxed applications. Something like - `RSVG_LOG_CONFIG=my_log_config.toml`. - -* With a configuration file, a la `~/.config/librsvg.toml`. Many - programs use librsvg and you don't want logs for all of them; allow - the configuration file to specify a process name, or maybe other - ways of determining when to log. For session programs like - gnome-shell, you can't easily set an environment variable to enable - logging - hence, a configuration file that only turns on logging - from the gnome-shell process. - -All of the above should be well documented, and then we can deprecate `RSVG_LOG`. - -## Which SVG caused a crash? - -Every once in a while, a bug report comes in like "$application -crashed in librsvg". The application renders many SVGs, often -indirectly via gdk-pixbuf, and it is hard to know exactly which SVG -caused the problem. Think of gnome-shell or gnome-software. - -For applications that call librsvg directly, if they pass the filename -or a GFile then it is not hard to find out the source SVG. - -But for those that feed bytes into librsvg, including those that use -it indirectly via gdk-pixbuf, librsvg has no knowledge of the -filename. We need to use the base_uri then, or see if the pixbuf -loader can be modified to propagate this information (is it even -available from the GdkPixbufLoader machinery?). - -If all else fails, we can have an exfiltration mechanism. How can we -avoid logging *all* the SVG data that gnome-shell renders, for -example? Configure the logger to skip the first N SVGs, and hope that -the order is deterministic? We can't really "log only if there is a -crash during rendering". - -Log only the checksums of SVGs or data lengths, and use that to find -which SVG caused the crash? I.e. have the user use a two-step process -to find a crash: get a log (written synchronously) of all SVG -checksums/lengths, and then reconfigure the logger to only exfiltrate -the last one that got logged - presumably that one caused the crash. - -## Which dynamically-created SVG caused a problem? - -Consider a bug like -https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5415 where an -application dynamically generates an SVG and feeds it to librsvg. -That bug was not a crash; it was about incorrect values returned from -an librsvg API function. For those cases it may be useful to be able -to exfiltrate an SVG and its stylesheets only if it matches a -user-provided substring. - -## Global configuration - -`$(XDG_CONFIG_HOME)/librsvg.toml` - for startup-like processes like -gnome-shell, for which it is hard to set an environment variable: - -## Per-process configuration - -`RSVG_LOG_CONFIG=my_log_config.toml my_process` - - - -## Programmatic API - -FIXME - - -## Configuration format - -```toml -[logging] -enabled=true -process=gnome-shell # mandatory for global config - don't want to log all processes - warn to g_log if key is not set -output=/home/username/rsvg.log # if missing, log to g_log only - or use a output_to_g_log=true instead? -``` - - -## API logging - -Log cr state at entry, surface type, starting transform. - -Log name/base_uri of rendered document. - -Can we know if it is a gresource? Or a byte buffer? Did it come from gdk-pixbuf? - -## Log contents - -/home/username/rsvg.log - json doesn't have comments; put one of these in a string somehow: - -``` - ****************************************************************************** - * This log file exists because you enabled logging in ~/.config/librsvg.toml * - * for the "gnome-shell" process. * - * * - * If you want to disable this kind of log, please turn it off in that file * - * or delete that file entirely. * - ****************************************************************************** - - ****************************************************************************** - * This log file exists because you enabled logging with * - * RSVG_LOG_CONFIG=config.toml for the "single-process-name" process. * - * * - * If you want to disable this kind of log, FIXME */ - ****************************************************************************** -``` - -** To-do list [0/1] - -- [ ] Audit code for GIO errors; log there. - -- [ ] Audit code for Cairo calls that yield errors; log there. - -- [ ] Log the entire ancestry of the element that caused the error? Is that an insta-reproducer? diff --git a/devel-docs/api_observability.rst b/devel-docs/api_observability.rst new file mode 100644 index 000000000..5a19b424b --- /dev/null +++ b/devel-docs/api_observability.rst @@ -0,0 +1,196 @@ +Observability +============= + +Librsvg supports basic, mostly ad-hoc logging with an ``RSVG_LOG=1`` +environment variable. This has not been very effective in letting me, +the maintainer, know what went wrong when someone reports a bug about +librsvg doing the wrong thing in an application. Part of it is because +the code could be more thorough about logging (e.g. log at all error +points), but also part of it is that there is no logging about what API +calls are made into the library. On each bug reported on behalf of a +particular application, my thought process goes something like this: + +- What was the app doing? + +- Can I obtain the problematic SVG? + +- Does the bug reporter even know what the problematic SVG was? + +- Was the app rendering with direct calls to the librsvg API? + +- Or was it using the gdk-pixbuf loader, and thus has very little + control of how librsvg is used? + +- If non-pixbuf, what was the Cairo state when librsvg was called? + +- What sort of interesting API calls are being used? Stylesheet + injection? Re-rendering single elements with different styles? + +And every time, I must ask the bug reporter for information related to +that, or to point me to the relevant source code where they were using +librsvg… which is not terribly useful, since building their code and +reproducing the bug with it is A Yak That Should Not Have To Be Shaved. + +Desiderata +---------- + +Know exactly what an application did with librsvg: + +- All API calls and their parameters. + +- State of the Cairo context at entry. + +- “What SVG?” - be careful and explicit about exfiltrating SVG data to + the logs. + +- Basic platform stuff? Is the platform triple enough? Distro ID? + +- Versions of dependencies. + +- Version of librsvg itself. + +Internals of the library: + +- Regular debug tracing. We may have options to enable/disable tracing + domains: parsing, cascading, referencing elements, temporary surfaces + during filtering, render tree, etc. + +- Log all points where an error is detected/generated, even if it will + be discarded later (e.g. invalid CSS values are silently ignored, per + the spec). + +Enabling logging +---------------- + +It may be useful to be able to enable logging in various ways: + +- Programmatically, for when one has control of the source code of the + problematic application. Enable logging at the problem spot, for the + SVG you know that exhibits the problem, and be done with it. This can + probably be at the individual ``RsvgHandle`` level, not globally. For + global logging within a single process, see the next point. + +- For a single process which one can easily launch via the command + line; e.g. with an environment variable. This works well for + non-sandboxed applications. Something like + ``RSVG_LOG_CONFIG=my_log_config.toml``. + +- With a configuration file, a la ``~/.config/librsvg.toml``. Many + programs use librsvg and you don’t want logs for all of them; allow + the configuration file to specify a process name, or maybe other ways + of determining when to log. For session programs like gnome-shell, + you can’t easily set an environment variable to enable logging - + hence, a configuration file that only turns on logging from the + gnome-shell process. + +All of the above should be well documented, and then we can deprecate +``RSVG_LOG``. + +Which SVG caused a crash? +------------------------- + +Every once in a while, a bug report comes in like “$application crashed +in librsvg”. The application renders many SVGs, often indirectly via +gdk-pixbuf, and it is hard to know exactly which SVG caused the problem. +Think of gnome-shell or gnome-software. + +For applications that call librsvg directly, if they pass the filename +or a GFile then it is not hard to find out the source SVG. + +But for those that feed bytes into librsvg, including those that use it +indirectly via gdk-pixbuf, librsvg has no knowledge of the filename. We +need to use the base_uri then, or see if the pixbuf loader can be +modified to propagate this information (is it even available from the +GdkPixbufLoader machinery?). + +If all else fails, we can have an exfiltration mechanism. How can we +avoid logging *all* the SVG data that gnome-shell renders, for example? +Configure the logger to skip the first N SVGs, and hope that the order +is deterministic? We can’t really “log only if there is a crash during +rendering”. + +Log only the checksums of SVGs or data lengths, and use that to find +which SVG caused the crash? I.e. have the user use a two-step process to +find a crash: get a log (written synchronously) of all SVG +checksums/lengths, and then reconfigure the logger to only exfiltrate +the last one that got logged - presumably that one caused the crash. + +Which dynamically-created SVG caused a problem? +----------------------------------------------- + +Consider a bug like +https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5415 where an +application dynamically generates an SVG and feeds it to librsvg. That +bug was not a crash; it was about incorrect values returned from an +librsvg API function. For those cases it may be useful to be able to +exfiltrate an SVG and its stylesheets only if it matches a user-provided +substring. + +Global configuration +-------------------- + +``$(XDG_CONFIG_HOME)/librsvg.toml`` - for startup-like processes like +gnome-shell, for which it is hard to set an environment variable: + +Per-process configuration +------------------------- + +``RSVG_LOG_CONFIG=my_log_config.toml my_process`` + +Programmatic API +---------------- + +FIXME + +Configuration format +-------------------- + +.. code:: toml + + [logging] + enabled=true + process=gnome-shell # mandatory for global config - don't want to log all processes - warn to g_log if key is not set + output=/home/username/rsvg.log # if missing, log to g_log only - or use a output_to_g_log=true instead? + +API logging +----------- + +Log cr state at entry, surface type, starting transform. + +Log name/base_uri of rendered document. + +Can we know if it is a gresource? Or a byte buffer? Did it come from +gdk-pixbuf? + +Log contents +------------ + +/home/username/rsvg.log - json doesn’t have comments; put one of these +in a string somehow: + +:: + + ****************************************************************************** + * This log file exists because you enabled logging in ~/.config/librsvg.toml * + * for the "gnome-shell" process. * + * * + * If you want to disable this kind of log, please turn it off in that file * + * or delete that file entirely. * + ****************************************************************************** + + ****************************************************************************** + * This log file exists because you enabled logging with * + * RSVG_LOG_CONFIG=config.toml for the "single-process-name" process. * + * * + * If you want to disable this kind of log, FIXME */ + ****************************************************************************** + +To-do list +---------- + +- Audit code for GIO errors; log there. + +- Audit code for Cairo calls that yield errors; log there. + +- Log the entire ancestry of the element that caused the error? Is + that an insta-reproducer? diff --git a/devel-docs/index.rst b/devel-docs/index.rst index ddb1f68b4..0d8f08d0a 100644 --- a/devel-docs/index.rst +++ b/devel-docs/index.rst @@ -8,6 +8,7 @@ Development guide for librsvg contributing ci text_layout + api_observability :maxdepth: 1 :caption: Contents: @@ -61,13 +62,12 @@ request. We can then discuss it before coding. This way we will have a sort of big-picture development history apart from commit messages. - :doc:`text_layout` +- :doc:`api_observability` See https://rustc-dev-guide.rust-lang.org/walkthrough.html, section Overview, to formalize the RFC process for features vs. drive-by contributions. -FIXME: link the md here. - Information for maintainers --------------------------- -- GitLab From 3c2669939ed7d41cab97648d253decd961cdd130 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 12:15:04 -0500 Subject: [PATCH 06/44] CairoContextState: capture the initial matrix Part-of: --- src/log.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/log.rs b/src/log.rs index 505ae0862..a8ec06142 100644 --- a/src/log.rs +++ b/src/log.rs @@ -29,13 +29,18 @@ pub fn log_enabled() -> bool { #[derive(Copy, Clone, Debug, PartialEq)] struct CairoContextState { surface_type: cairo::SurfaceType, + matrix: cairo::Matrix, } impl CairoContextState { fn new(cr: &cairo::Context) -> Self { let surface_type = cr.target().type_(); + let matrix = cr.matrix(); - Self { surface_type } + Self { + surface_type, + matrix, + } } } @@ -44,7 +49,7 @@ mod tests { use super::*; #[test] - fn captures_cr_surface_type() { + fn captures_cr_state() { let surface = cairo::ImageSurface::create(cairo::Format::ARgb32, 10, 10).unwrap(); let cr = cairo::Context::new(&surface).unwrap(); let state = CairoContextState::new(&cr); @@ -52,17 +57,23 @@ mod tests { assert_eq!( CairoContextState { surface_type: cairo::SurfaceType::Image, + matrix: cairo::Matrix::identity(), }, state, ); let surface = cairo::RecordingSurface::create(cairo::Content::ColorAlpha, None).unwrap(); let cr = cairo::Context::new(&surface).unwrap(); + cr.scale(2.0, 3.0); let state = CairoContextState::new(&cr); + let mut matrix = cairo::Matrix::identity(); + matrix.scale(2.0, 3.0); + assert_eq!( CairoContextState { surface_type: cairo::SurfaceType::Recording, + matrix, }, state, ); -- GitLab From dc6b58d55ef74b6e0537b309052c2bba21f87ccd Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 14:38:35 -0500 Subject: [PATCH 07/44] session.rs: new file with a Session type, to track metadata during loading/rendering Part-of: --- Makefile.am | 1 + src/lib.rs | 1 + src/session.rs | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 src/session.rs diff --git a/Makefile.am b/Makefile.am index 58c2a5c9f..b962614c9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -98,6 +98,7 @@ LIBRSVG_SRC = \ src/property_defs.rs \ src/property_macros.rs \ src/rect.rs \ + src/session.rs \ src/shapes.rs \ src/space.rs \ src/structure.rs \ diff --git a/src/lib.rs b/src/lib.rs index 3dfb39f46..ce602d444 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,6 +209,7 @@ mod pattern; mod properties; mod property_defs; mod rect; +mod session; mod shapes; mod space; mod structure; diff --git a/src/session.rs b/src/session.rs new file mode 100644 index 000000000..8988f2fd2 --- /dev/null +++ b/src/session.rs @@ -0,0 +1,20 @@ +//! Tracks metadata for a loading/rendering session. + +use crate::log; + +/// Metadata for a loading/rendering session. +/// +/// When the calling program first uses one of the API entry points (e.g. `Loader::new()` +/// in the Rust API, or `rsvg_handle_new()` in the C API), there is no context yet where +/// librsvg's code may start to track things. This struct provides that context. +pub struct Session { + log_enabled: bool, +} + +impl Session { + pub fn new() -> Self { + Self { + log_enabled: log::log_enabled(), + } + } +} -- GitLab From d7af0a2d78f93480b2d6176dd82492c0a9c67a40 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 14:41:41 -0500 Subject: [PATCH 08/44] Loader: create a Session, even if it does not do anything yet Part-of: --- src/api.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/api.rs b/src/api.rs index 0ab0c51db..20e3aec6b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -13,6 +13,7 @@ pub use crate::{ use url::Url; use std::path::Path; +use std::sync::Arc; use gio::prelude::*; // Re-exposes glib's prelude as well use gio::Cancellable; @@ -20,6 +21,7 @@ use gio::Cancellable; use crate::{ dpi::Dpi, handle::{Handle, LoadOptions}, + session::Session, url_resolver::UrlResolver, }; @@ -31,10 +33,10 @@ use crate::{ /// of `Loader` in sequence to configure how SVG data should be /// loaded, and finally use one of the loading functions to load an /// [`SvgHandle`]. -#[derive(Default)] pub struct Loader { unlimited_size: bool, keep_image_data: bool, + session: Arc, } impl Loader { @@ -58,7 +60,19 @@ impl Loader { /// .unwrap(); /// ``` pub fn new() -> Self { - Self::default() + Self::new_with_session(Arc::new(Session::new())) + } + + /// Creates a `Loader` from a pre-created [`Session`]. + /// + /// This is useful when a `Loader` must be created by the C API, which should already + /// have created a session for logging. + pub(crate) fn new_with_session(session: Arc) -> Self { + Self { + unlimited_size: false, + keep_image_data: false, + session, + } } /// Controls safety limits used in the XML parser. -- GitLab From e0e90101d82dc4f6c305d62f3a26355a473db494 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 15:26:36 -0500 Subject: [PATCH 09/44] SvgHandle: turn into a real struct, from a tuple struct Part-of: --- src/api.rs | 34 +++++++++++++++++++--------------- src/c_api/sizing.rs | 2 +- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/api.rs b/src/api.rs index 20e3aec6b..7a35123b0 100644 --- a/src/api.rs +++ b/src/api.rs @@ -212,11 +212,13 @@ impl Loader { .with_unlimited_size(self.unlimited_size) .keep_image_data(self.keep_image_data); - Ok(SvgHandle(Handle::from_stream( - &load_options, - stream.as_ref(), - cancellable.map(|c| c.as_ref()), - )?)) + Ok(SvgHandle { + handle: Handle::from_stream( + &load_options, + stream.as_ref(), + cancellable.map(|c| c.as_ref()), + )?, + }) } } @@ -228,7 +230,9 @@ fn url_from_file(file: &gio::File) -> Result { /// /// You can create this from one of the `read` methods in /// [`Loader`]. -pub struct SvgHandle(pub(crate) Handle); +pub struct SvgHandle { + pub(crate) handle: Handle, +} impl SvgHandle { /// Checks if the SVG has an element with the specified `id`. @@ -239,7 +243,7 @@ impl SvgHandle { /// The purpose of the `Err()` case in the return value is to indicate an /// incorrectly-formatted `id` argument. pub fn has_element_with_id(&self, id: &str) -> Result { - self.0.has_sub(id) + self.handle.has_sub(id) } /// Sets a CSS stylesheet to use for an SVG document. @@ -251,7 +255,7 @@ impl SvgHandle { /// /// [origin]: https://drafts.csswg.org/css-cascade-3/#cascading-origins pub fn set_stylesheet(&mut self, css: &str) -> Result<(), LoadingError> { - self.0.set_stylesheet(css) + self.handle.set_stylesheet(css) } } @@ -364,7 +368,7 @@ impl<'a> CairoRenderer<'a> { /// [`render_document`]: #method.render_document /// [`intrinsic_size_in_pixels`]: #method.intrinsic_size_in_pixels pub fn intrinsic_dimensions(&self) -> IntrinsicDimensions { - let d = self.handle.0.get_intrinsic_dimensions(); + let d = self.handle.handle.get_intrinsic_dimensions(); IntrinsicDimensions { width: Into::into(d.width), @@ -397,7 +401,7 @@ impl<'a> CairoRenderer<'a> { return None; } - Some(self.handle.0.width_height_to_user(self.dpi)) + Some(self.handle.handle.width_height_to_user(self.dpi)) } /// Renders the whole SVG document fitted to a viewport @@ -414,7 +418,7 @@ impl<'a> CairoRenderer<'a> { viewport: &cairo::Rectangle, ) -> Result<(), RenderingError> { self.handle - .0 + .handle .render_document(cr, viewport, &self.user_language, self.dpi, self.is_testing) } @@ -448,7 +452,7 @@ impl<'a> CairoRenderer<'a> { viewport: &cairo::Rectangle, ) -> Result<(cairo::Rectangle, cairo::Rectangle), RenderingError> { self.handle - .0 + .handle .get_geometry_for_layer(id, viewport, &self.user_language, self.dpi, self.is_testing) .map(|(i, l)| (i, l)) } @@ -478,7 +482,7 @@ impl<'a> CairoRenderer<'a> { id: Option<&str>, viewport: &cairo::Rectangle, ) -> Result<(), RenderingError> { - self.handle.0.render_layer( + self.handle.handle.render_layer( cr, id, viewport, @@ -523,7 +527,7 @@ impl<'a> CairoRenderer<'a> { id: Option<&str>, ) -> Result<(cairo::Rectangle, cairo::Rectangle), RenderingError> { self.handle - .0 + .handle .get_geometry_for_element(id, &self.user_language, self.dpi, self.is_testing) .map(|(i, l)| (i, l)) } @@ -550,7 +554,7 @@ impl<'a> CairoRenderer<'a> { id: Option<&str>, element_viewport: &cairo::Rectangle, ) -> Result<(), RenderingError> { - self.handle.0.render_element( + self.handle.handle.render_element( cr, id, element_viewport, diff --git a/src/c_api/sizing.rs b/src/c_api/sizing.rs index 902c188f7..e0c2090a0 100644 --- a/src/c_api/sizing.rs +++ b/src/c_api/sizing.rs @@ -51,7 +51,7 @@ impl<'a> LegacySize for CairoRenderer<'a> { let size_from_intrinsic_dimensions = self.intrinsic_size_in_pixels().or_else(|| { size_in_pixels_from_percentage_width_and_height( - &self.handle.0, + &self.handle.handle, &self.intrinsic_dimensions(), self.dpi, ) -- GitLab From 23dc7b1a254237eb85f1e54319e1772dc164fd81 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 15:27:56 -0500 Subject: [PATCH 10/44] SvgHandle: store a Session, too It gets passed when the Loader creates the SvgHandle. Part-of: --- src/api.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/api.rs b/src/api.rs index 7a35123b0..c63fa9ea4 100644 --- a/src/api.rs +++ b/src/api.rs @@ -218,6 +218,7 @@ impl Loader { stream.as_ref(), cancellable.map(|c| c.as_ref()), )?, + session: self.session, }) } } @@ -231,6 +232,7 @@ fn url_from_file(file: &gio::File) -> Result { /// You can create this from one of the `read` methods in /// [`Loader`]. pub struct SvgHandle { + session: Arc, pub(crate) handle: Handle, } -- GitLab From 4231dc23e806bf53f1a1304851421d1a1e90bb07 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Fri, 19 Aug 2022 16:58:15 -0500 Subject: [PATCH 11/44] Handle: store a Session Part-of: --- src/api.rs | 1 + src/handle.rs | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/api.rs b/src/api.rs index c63fa9ea4..b8ea4e879 100644 --- a/src/api.rs +++ b/src/api.rs @@ -214,6 +214,7 @@ impl Loader { Ok(SvgHandle { handle: Handle::from_stream( + self.session.clone(), &load_options, stream.as_ref(), cancellable.map(|c| c.as_ref()), diff --git a/src/handle.rs b/src/handle.rs index 2480eb9a8..541e38777 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -2,6 +2,8 @@ //! //! This module provides the primitives on which the public APIs are implemented. +use std::sync::Arc; + use crate::accept_language::UserLanguage; use crate::bbox::BoundingBox; use crate::css::{Origin, Stylesheet}; @@ -12,6 +14,7 @@ use crate::error::{DefsLookupErrorKind, LoadingError, RenderingError}; use crate::length::*; use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::rect::Rect; +use crate::session::Session; use crate::structure::IntrinsicDimensions; use crate::url_resolver::{AllowedUrl, UrlResolver}; @@ -79,17 +82,20 @@ impl LoadOptions { /// /// [`from_stream`]: #method.from_stream pub struct Handle { + session: Arc, document: Document, } impl Handle { /// Loads an SVG document into a `Handle`. pub fn from_stream( + session: Arc, load_options: &LoadOptions, stream: &gio::InputStream, cancellable: Option<&gio::Cancellable>, ) -> Result { Ok(Handle { + session, document: Document::load_from_stream(load_options, stream, cancellable)?, }) } -- GitLab From 2e5b95385838682aa3fe0edd119f2cb39a816f79 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 13:45:21 -0500 Subject: [PATCH 12/44] Session: move the fields to an inner Arc It's going to be miserable to deref this everywhere, especially once we start having a mutex in there. Part-of: --- src/api.rs | 9 ++++----- src/handle.rs | 6 ++---- src/session.rs | 15 ++++++++++++++- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/api.rs b/src/api.rs index b8ea4e879..33729999b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -13,7 +13,6 @@ pub use crate::{ use url::Url; use std::path::Path; -use std::sync::Arc; use gio::prelude::*; // Re-exposes glib's prelude as well use gio::Cancellable; @@ -36,7 +35,7 @@ use crate::{ pub struct Loader { unlimited_size: bool, keep_image_data: bool, - session: Arc, + session: Session, } impl Loader { @@ -60,14 +59,14 @@ impl Loader { /// .unwrap(); /// ``` pub fn new() -> Self { - Self::new_with_session(Arc::new(Session::new())) + Self::new_with_session(Session::new()) } /// Creates a `Loader` from a pre-created [`Session`]. /// /// This is useful when a `Loader` must be created by the C API, which should already /// have created a session for logging. - pub(crate) fn new_with_session(session: Arc) -> Self { + pub(crate) fn new_with_session(session: Session) -> Self { Self { unlimited_size: false, keep_image_data: false, @@ -233,7 +232,7 @@ fn url_from_file(file: &gio::File) -> Result { /// You can create this from one of the `read` methods in /// [`Loader`]. pub struct SvgHandle { - session: Arc, + session: Session, pub(crate) handle: Handle, } diff --git a/src/handle.rs b/src/handle.rs index 541e38777..99827816b 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -2,8 +2,6 @@ //! //! This module provides the primitives on which the public APIs are implemented. -use std::sync::Arc; - use crate::accept_language::UserLanguage; use crate::bbox::BoundingBox; use crate::css::{Origin, Stylesheet}; @@ -82,14 +80,14 @@ impl LoadOptions { /// /// [`from_stream`]: #method.from_stream pub struct Handle { - session: Arc, + session: Session, document: Document, } impl Handle { /// Loads an SVG document into a `Handle`. pub fn from_stream( - session: Arc, + session: Session, load_options: &LoadOptions, stream: &gio::InputStream, cancellable: Option<&gio::Cancellable>, diff --git a/src/session.rs b/src/session.rs index 8988f2fd2..e363e8e7f 100644 --- a/src/session.rs +++ b/src/session.rs @@ -1,5 +1,7 @@ //! Tracks metadata for a loading/rendering session. +use std::sync::Arc; + use crate::log; /// Metadata for a loading/rendering session. @@ -7,14 +9,25 @@ use crate::log; /// When the calling program first uses one of the API entry points (e.g. `Loader::new()` /// in the Rust API, or `rsvg_handle_new()` in the C API), there is no context yet where /// librsvg's code may start to track things. This struct provides that context. +#[derive(Clone)] pub struct Session { + inner: Arc, +} + +struct SessionInner { log_enabled: bool, } impl Session { pub fn new() -> Self { Self { - log_enabled: log::log_enabled(), + inner: Arc::new(SessionInner { + log_enabled: log::log_enabled(), + }), } } + + pub fn log_enabled(&self) -> bool { + self.inner.log_enabled + } } -- GitLab From e372e716f45aa31539ea01c50e9864d24e220567 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 13:48:00 -0500 Subject: [PATCH 13/44] rsvg_log_session!(): new macro, uses a Session to decide whether to log We'll gradually replace calls to rsvg_log!() with this other macro, and then rename all of them in a single shot. Part-of: --- src/handle.rs | 3 ++- src/log.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/handle.rs b/src/handle.rs index 99827816b..e9d05e209 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -208,7 +208,8 @@ impl Handle { .lookup_internal_node(&id) .ok_or(DefsLookupErrorKind::NotFound), NodeId::External(_, _) => { - rsvg_log!( + rsvg_log_session!( + self.session, "the public API is not allowed to look up external references: {}", node_id ); diff --git a/src/log.rs b/src/log.rs index a8ec06142..4505e9737 100644 --- a/src/log.rs +++ b/src/log.rs @@ -13,6 +13,18 @@ macro_rules! rsvg_log { }; } +#[macro_export] +macro_rules! rsvg_log_session { + ( + $session:expr, + $($arg:tt)+ + ) => { + if $session.log_enabled() { + println!("{}", format_args!($($arg)+)); + } + }; +} + pub fn log_enabled() -> bool { static ENABLED: Lazy = Lazy::new(|| ::std::env::var_os("RSVG_LOG").is_some()); -- GitLab From eb1f7381f3ac1ac12ca532243607797a384cf3ee Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 15:31:02 -0500 Subject: [PATCH 14/44] Session.new_for_test_suite(): New method Part-of: --- src/session.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/session.rs b/src/session.rs index e363e8e7f..d8e2f9bda 100644 --- a/src/session.rs +++ b/src/session.rs @@ -27,6 +27,15 @@ impl Session { } } + #[cfg(test)] + pub fn new_for_test_suite() -> Self { + Self { + inner: Arc::new(SessionInner { + log_enabled: false, + }), + } + } + pub fn log_enabled(&self) -> bool { self.inner.log_enabled } -- GitLab From d0c3d5a04f94429f0825862c740bd1bcc437e3aa Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 15:31:12 -0500 Subject: [PATCH 15/44] Pass a Session around the document loading code Part-of: --- src/api.rs | 10 +++++++--- src/document.rs | 25 +++++++++++++++++++++---- src/handle.rs | 9 +++++++-- src/session.rs | 4 +--- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/api.rs b/src/api.rs index 33729999b..990ee7443 100644 --- a/src/api.rs +++ b/src/api.rs @@ -419,9 +419,13 @@ impl<'a> CairoRenderer<'a> { cr: &cairo::Context, viewport: &cairo::Rectangle, ) -> Result<(), RenderingError> { - self.handle - .handle - .render_document(cr, viewport, &self.user_language, self.dpi, self.is_testing) + self.handle.handle.render_document( + cr, + viewport, + &self.user_language, + self.dpi, + self.is_testing, + ) } /// Computes the (ink_rect, logical_rect) of an SVG element, as if diff --git a/src/document.rs b/src/document.rs index f682f7460..fb9071ef2 100644 --- a/src/document.rs +++ b/src/document.rs @@ -18,6 +18,7 @@ use crate::handle::LoadOptions; use crate::io::{self, BinaryData}; use crate::limits; use crate::node::{Node, NodeBorrow, NodeData}; +use crate::session::Session; use crate::surface_utils::shared_surface::SharedImageSurface; use crate::url_resolver::{AllowedUrl, UrlResolver}; use crate::xml::{xml_load_from_possibly_compressed_stream, Attributes}; @@ -72,6 +73,9 @@ pub struct Document { /// Tree of nodes; the root is guaranteed to be an `` element. tree: Node, + /// Metadata about the SVG handle. + session: Session, + /// Mapping from `id` attributes to nodes. ids: HashMap, @@ -94,12 +98,13 @@ pub struct Document { impl Document { /// Constructs a `Document` by loading it from a stream. pub fn load_from_stream( + session: Session, load_options: &LoadOptions, stream: &gio::InputStream, cancellable: Option<&gio::Cancellable>, ) -> Result { xml_load_from_possibly_compressed_stream( - DocumentBuilder::new(load_options), + DocumentBuilder::new(session, load_options), load_options.unlimited_size, stream, cancellable, @@ -115,6 +120,7 @@ impl Document { let stream = gio::MemoryInputStream::from_bytes(&bytes); Document::load_from_stream( + Session::new_for_test_suite(), &LoadOptions::new(UrlResolver::new(None)), &stream.upcast(), None::<&gio::Cancellable>, @@ -134,7 +140,7 @@ impl Document { NodeId::External(url, id) => self .externs .borrow_mut() - .lookup(&self.load_options, url, id) + .lookup(&self.session, &self.load_options, url, id) .ok(), } } @@ -177,16 +183,18 @@ impl Resources { pub fn lookup( &mut self, + session: &Session, load_options: &LoadOptions, url: &str, id: &str, ) -> Result { - self.get_extern_document(load_options, url) + self.get_extern_document(session, load_options, url) .and_then(|doc| doc.lookup_internal_node(id).ok_or(LoadingError::BadUrl)) } fn get_extern_document( &mut self, + session: &Session, load_options: &LoadOptions, href: &str, ) -> Result, LoadingError> { @@ -204,6 +212,7 @@ impl Resources { .map_err(LoadingError::from) .and_then(|stream| { Document::load_from_stream( + session.clone(), &load_options.copy_with_base_url(aurl), &stream, None, @@ -477,6 +486,7 @@ impl NodeStack { } pub struct DocumentBuilder { + session: Session, load_options: LoadOptions, tree: Option, ids: HashMap, @@ -484,8 +494,9 @@ pub struct DocumentBuilder { } impl DocumentBuilder { - pub fn new(load_options: &LoadOptions) -> DocumentBuilder { + pub fn new(session: Session, load_options: &LoadOptions) -> DocumentBuilder { DocumentBuilder { + session, load_options: load_options.clone(), tree: None, ids: HashMap::new(), @@ -493,6 +504,10 @@ impl DocumentBuilder { } } + pub fn session(&self) -> &Session { + &self.session + } + pub fn append_stylesheet_from_xml_processing_instruction( &mut self, alternate: Option, @@ -575,6 +590,7 @@ impl DocumentBuilder { pub fn build(self) -> Result { let DocumentBuilder { load_options, + session, tree, ids, stylesheets, @@ -586,6 +602,7 @@ impl DocumentBuilder { if is_element_of_type!(root, Svg) { let mut document = Document { tree: root, + session, ids, externs: RefCell::new(Resources::new()), images: RefCell::new(Images::new()), diff --git a/src/handle.rs b/src/handle.rs index e9d05e209..13e670d5b 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -93,8 +93,13 @@ impl Handle { cancellable: Option<&gio::Cancellable>, ) -> Result { Ok(Handle { - session, - document: Document::load_from_stream(load_options, stream, cancellable)?, + session: session.clone(), + document: Document::load_from_stream( + session.clone(), + load_options, + stream, + cancellable, + )?, }) } diff --git a/src/session.rs b/src/session.rs index d8e2f9bda..87385aa8a 100644 --- a/src/session.rs +++ b/src/session.rs @@ -30,9 +30,7 @@ impl Session { #[cfg(test)] pub fn new_for_test_suite() -> Self { Self { - inner: Arc::new(SessionInner { - log_enabled: false, - }), + inner: Arc::new(SessionInner { log_enabled: false }), } } -- GitLab From 2f8c13263c45760ae92f8c9ab2c10031da18157f Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 15:41:05 -0500 Subject: [PATCH 16/44] xml/mod.rs: Use a session for logging Part-of: --- src/xml/mod.rs | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/xml/mod.rs b/src/xml/mod.rs index 8a6c7ed40..7fb4e147e 100644 --- a/src/xml/mod.rs +++ b/src/xml/mod.rs @@ -268,6 +268,8 @@ impl XmlState { let mut inner = self.inner.borrow_mut(); + let session = inner.document_builder.as_mut().unwrap().session().clone(); + if let Some(href) = href { // FIXME: https://www.w3.org/TR/xml-stylesheet/ does not seem to specify // what to do if the stylesheet cannot be loaded, so here we ignore the error. @@ -278,13 +280,17 @@ impl XmlState { .append_stylesheet_from_xml_processing_instruction(alternate, type_, &href) .is_err() { - rsvg_log!( + rsvg_log_session!( + session, "invalid xml-stylesheet {} in XML processing instruction", href ); } } else { - rsvg_log!("xml-stylesheet processing instruction does not have href; ignoring"); + rsvg_log_session!( + session, + "xml-stylesheet processing instruction does not have href; ignoring" + ); } } else { self.error(LoadingError::XmlParseError(String::from( @@ -477,6 +483,15 @@ impl XmlState { encoding: Option<&str>, ) -> Result<(), AcquireError> { if let Some(href) = href { + let session = self + .inner + .borrow() + .document_builder + .as_ref() + .unwrap() + .session() + .clone(); + let aurl = self .inner .borrow() @@ -487,7 +502,7 @@ impl XmlState { .map_err(|e| { // FIXME: should AlloweUrlError::UrlParseError be a fatal error, // not a resource error? - rsvg_log!("could not acquire \"{}\": {}", href, e); + rsvg_log_session!(session, "could not acquire \"{}\": {}", href, e); AcquireError::ResourceError })?; @@ -517,8 +532,17 @@ impl XmlState { } fn acquire_text(&self, aurl: &AllowedUrl, encoding: Option<&str>) -> Result<(), AcquireError> { + let session = self + .inner + .borrow() + .document_builder + .as_ref() + .unwrap() + .session() + .clone(); + let binary = io::acquire_data(aurl, None).map_err(|e| { - rsvg_log!("could not acquire \"{}\": {}", aurl, e); + rsvg_log_session!(session, "could not acquire \"{}\": {}", aurl, e); AcquireError::ResourceError })?; -- GitLab From 63731fd6cfb4510c41490de98bf11b2889845dd8 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 18:29:01 -0500 Subject: [PATCH 17/44] DrawingCtx: use a session for logging Part-of: --- src/drawing_ctx.rs | 26 ++++++++++++++++++++------ src/handle.rs | 4 ++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs index cde3c61d3..3a44fd77f 100644 --- a/src/drawing_ctx.rs +++ b/src/drawing_ctx.rs @@ -37,6 +37,7 @@ use crate::properties::{ Overflow, PaintTarget, ShapeRendering, StrokeLinecap, StrokeLinejoin, TextRendering, }; use crate::rect::{IRect, Rect}; +use crate::session::Session; use crate::surface_utils::{ shared_surface::ExclusiveImageSurface, shared_surface::SharedImageSurface, shared_surface::SurfaceType, @@ -169,6 +170,8 @@ struct Viewport { } pub struct DrawingCtx { + session: Session, + initial_viewport: Viewport, dpi: Dpi, @@ -196,6 +199,7 @@ pub enum DrawingMode { /// /// This creates a DrawingCtx internally and starts drawing at the specified `node`. pub fn draw_tree( + session: Session, mode: DrawingMode, cr: &cairo::Context, viewport: Rect, @@ -239,6 +243,7 @@ pub fn draw_tree( let viewport = viewport.translate((-viewport.x0, -viewport.y0)); let mut draw_ctx = DrawingCtx::new( + session, cr, transform, viewport, @@ -281,6 +286,7 @@ const CAIRO_TAG_LINK: &str = "Link"; impl DrawingCtx { fn new( + session: Session, cr: &cairo::Context, transform: Transform, viewport: Rect, @@ -296,6 +302,7 @@ impl DrawingCtx { let viewport_stack = vec![initial_viewport]; DrawingCtx { + session, initial_viewport, dpi, cr_stack: Rc::new(RefCell::new(Vec::new())), @@ -320,6 +327,7 @@ impl DrawingCtx { cr_stack.borrow_mut().push(self.cr.clone()); DrawingCtx { + session: self.session.clone(), initial_viewport: self.initial_viewport, dpi: self.dpi, cr_stack, @@ -488,7 +496,8 @@ impl DrawingCtx { "viewport_to_viewbox_transform only returns errors when vbox != None" ), Some(v) => { - rsvg_log!( + rsvg_log_session!( + self.session, "ignoring viewBox ({}, {}, {}, {}) since it is not usable", v.x0, v.y0, @@ -575,7 +584,7 @@ impl DrawingCtx { Ok(n) => n, Err(AcquireError::CircularReference(_)) => { - rsvg_log!("circular reference in element {}", mask_node); + rsvg_log_session!(self.session, "circular reference in element {}", mask_node); return Ok(None); } @@ -1036,7 +1045,7 @@ impl DrawingCtx { Ok(n) => n, Err(AcquireError::CircularReference(ref node)) => { - rsvg_log!("circular reference in element {}", node); + rsvg_log_session!(self.session, "circular reference in element {}", node); return Ok(false); } @@ -1611,7 +1620,7 @@ impl DrawingCtx { Ok(n) => n, Err(AcquireError::CircularReference(_)) => { - rsvg_log!("circular reference in element {}", node); + rsvg_log_session!(self.session, "circular reference in element {}", node); return Ok(self.empty_bbox()); } @@ -1622,7 +1631,7 @@ impl DrawingCtx { Ok(acquired) => acquired, Err(AcquireError::CircularReference(node)) => { - rsvg_log!("circular reference in element {}", node); + rsvg_log_session!(self.session, "circular reference in element {}", node); return Ok(self.empty_bbox()); } @@ -1635,7 +1644,12 @@ impl DrawingCtx { Err(AcquireError::InvalidLinkType(_)) => unreachable!(), Err(AcquireError::LinkNotFound(node_id)) => { - rsvg_log!("element {} references nonexistent \"{}\"", node, node_id); + rsvg_log_session!( + self.session, + "element {} references nonexistent \"{}\"", + node, + node_id + ); return Ok(self.empty_bbox()); } }; diff --git a/src/handle.rs b/src/handle.rs index 13e670d5b..8dec2a880 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -160,6 +160,7 @@ impl Handle { let cr = cairo::Context::new(&target)?; let bbox = draw_tree( + self.session.clone(), DrawingMode::LimitToStack { node, root }, &cr, viewport, @@ -253,6 +254,7 @@ impl Handle { with_saved_cr(cr, || { draw_tree( + self.session.clone(), DrawingMode::LimitToStack { node, root }, cr, viewport, @@ -279,6 +281,7 @@ impl Handle { let node = node.clone(); draw_tree( + self.session.clone(), DrawingMode::OnlyNode(node), &cr, unit_rectangle(), @@ -351,6 +354,7 @@ impl Handle { cr.translate(-ink_r.x0, -ink_r.y0); draw_tree( + self.session.clone(), DrawingMode::OnlyNode(node), cr, unit_rectangle(), -- GitLab From 156f1dabc7d92e23b8744be1555bfbdfd869ba86 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 18:59:24 -0500 Subject: [PATCH 18/44] DrawingCtx: expose a session() method so it can be used around Part-of: --- src/drawing_ctx.rs | 4 ++++ src/marker.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs index 3a44fd77f..93a6e03d7 100644 --- a/src/drawing_ctx.rs +++ b/src/drawing_ctx.rs @@ -340,6 +340,10 @@ impl DrawingCtx { } } + pub fn session(&self) -> &Session { + &self.session + } + pub fn user_language(&self) -> &UserLanguage { &self.user_language } diff --git a/src/marker.rs b/src/marker.rs index ee438201a..5acab183f 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -596,7 +596,7 @@ fn emit_marker_by_node( } Err(e) => { - rsvg_log!("could not acquire marker: {}", e); + rsvg_log_session!(draw_ctx.session(), "could not acquire marker: {}", e); Ok(draw_ctx.empty_bbox()) } } -- GitLab From 05adeaec2d5cd3b6580819c1a72cd02ae40a71f4 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 19:07:22 -0500 Subject: [PATCH 19/44] shapes.rs: Use a session for logging where we have a DrawingCtx Still to change is the implementation of Path::set_attributes() - but we don't have a DrawingCtx at that stage. Maybe we need to pass the session to all the set_attributes() functions. Part-of: --- src/shapes.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/shapes.rs b/src/shapes.rs index 35716c93e..7cc8c5bba 100644 --- a/src/shapes.rs +++ b/src/shapes.rs @@ -18,6 +18,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::parsers::{optional_comma, Parse, ParseValue}; use crate::path_builder::{LargeArc, Path as SvgPath, PathBuilder, Sweep}; use crate::properties::ComputedValues; +use crate::session::Session; use crate::xml::Attributes; #[derive(PartialEq)] @@ -86,10 +87,15 @@ macro_rules! impl_draw { let marker_mid_node; let marker_end_node; + let session = draw_ctx.session(); + if shape_def.markers == Markers::Yes { - marker_start_node = acquire_marker(acquired_nodes, &values.marker_start().0); - marker_mid_node = acquire_marker(acquired_nodes, &values.marker_mid().0); - marker_end_node = acquire_marker(acquired_nodes, &values.marker_end().0); + marker_start_node = + acquire_marker(session, acquired_nodes, &values.marker_start().0); + marker_mid_node = + acquire_marker(session, acquired_nodes, &values.marker_mid().0); + marker_end_node = + acquire_marker(session, acquired_nodes, &values.marker_end().0); } else { marker_start_node = None; marker_mid_node = None; @@ -146,12 +152,16 @@ macro_rules! impl_draw { }; } -fn acquire_marker(acquired_nodes: &mut AcquiredNodes<'_>, iri: &Iri) -> Option { +fn acquire_marker( + session: &Session, + acquired_nodes: &mut AcquiredNodes<'_>, + iri: &Iri, +) -> Option { iri.get().and_then(|id| { acquired_nodes .acquire(id) .map_err(|e| { - rsvg_log!("cannot render marker: {}", e); + rsvg_log_session!(session, "cannot render marker: {}", e); }) .ok() .and_then(|acquired| { @@ -160,7 +170,7 @@ fn acquire_marker(acquired_nodes: &mut AcquiredNodes<'_>, iri: &Iri) -> Option Date: Mon, 22 Aug 2022 19:10:14 -0500 Subject: [PATCH 20/44] element.rs: Use the DrawingCtx.session() Part-of: --- src/element.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/element.rs b/src/element.rs index 4619cda8e..ffa47fb08 100644 --- a/src/element.rs +++ b/src/element.rs @@ -276,7 +276,11 @@ impl Draw for ElementInner { Ok(draw_ctx.empty_bbox()) } } else { - rsvg_log!("(not rendering element {} because it is in error)", self); + rsvg_log_session!( + draw_ctx.session(), + "(not rendering element {} because it is in error)", + self + ); // maybe we should actually return a RenderingError::ElementIsInError here? Ok(draw_ctx.empty_bbox()) -- GitLab From 2c2f0d1b997f58664d9932a1d7c41a49e38e7929 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 19:20:24 -0500 Subject: [PATCH 21/44] Pass a session to the PaintServer.resolve() functions Part-of: --- src/drawing_ctx.rs | 2 ++ src/paint_server.rs | 4 +++- src/pattern.rs | 4 +++- src/shapes.rs | 6 ++++-- src/structure.rs | 2 ++ src/text.rs | 20 ++++++++++++++++++-- 6 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs index 93a6e03d7..9193d1429 100644 --- a/src/drawing_ctx.rs +++ b/src/drawing_ctx.rs @@ -806,6 +806,7 @@ impl DrawingCtx { current_color, None, None, + self.session(), ) .to_user_space(&bbox, ¶ms, values), ); @@ -820,6 +821,7 @@ impl DrawingCtx { current_color, None, None, + self.session(), ) .to_user_space(&bbox, ¶ms, values), ); diff --git a/src/paint_server.rs b/src/paint_server.rs index a744c7286..bc699dae3 100644 --- a/src/paint_server.rs +++ b/src/paint_server.rs @@ -12,6 +12,7 @@ use crate::node::NodeBorrow; use crate::parsers::Parse; use crate::pattern::{ResolvedPattern, UserSpacePattern}; use crate::properties::ComputedValues; +use crate::session::Session; use crate::unit_interval::UnitInterval; use crate::util; @@ -126,6 +127,7 @@ impl PaintServer { current_color: cssparser::RGBA, context_fill: Option, context_stroke: Option, + session: &Session, ) -> PaintSource { match self { PaintServer::Iri { @@ -147,7 +149,7 @@ impl PaintServer { }) } Element::Pattern(ref p) => { - p.resolve(node, acquired_nodes, opacity).map(|p| { + p.resolve(node, acquired_nodes, opacity, session).map(|p| { PaintSource::Pattern( p, alternate.map(|c| resolve_color(&c, opacity, current_color)), diff --git a/src/pattern.rs b/src/pattern.rs index 42f6aeb3a..e14bb3e29 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -15,6 +15,7 @@ use crate::node::{Node, NodeBorrow, WeakNode}; use crate::parsers::ParseValue; use crate::properties::ComputedValues; use crate::rect::Rect; +use crate::session::Session; use crate::transform::{Transform, TransformAttribute}; use crate::unit_interval::UnitInterval; use crate::viewbox::*; @@ -436,6 +437,7 @@ impl Pattern { node: &Node, acquired_nodes: &mut AcquiredNodes<'_>, opacity: UnitInterval, + session: &Session, ) -> Result { let Unresolved { mut pattern, @@ -471,7 +473,7 @@ impl Pattern { } Err(e) => { - rsvg_log!("Stopping pattern resolution: {}", e); + rsvg_log_session!(session, "Stopping pattern resolution: {}", e); pattern = pattern.resolve_from_defaults(); break; } diff --git a/src/shapes.rs b/src/shapes.rs index 7cc8c5bba..b617da9e8 100644 --- a/src/shapes.rs +++ b/src/shapes.rs @@ -63,12 +63,15 @@ macro_rules! impl_draw { let stroke = Stroke::new(values, ¶ms); + let session = draw_ctx.session(); + let stroke_paint = values.stroke().0.resolve( acquired_nodes, values.stroke_opacity().0, values.color().0, cascaded.context_fill.clone(), cascaded.context_stroke.clone(), + session, ); let fill_paint = values.fill().0.resolve( @@ -77,6 +80,7 @@ macro_rules! impl_draw { values.color().0, cascaded.context_fill.clone(), cascaded.context_stroke.clone(), + session, ); let fill_rule = values.fill_rule(); @@ -87,8 +91,6 @@ macro_rules! impl_draw { let marker_mid_node; let marker_end_node; - let session = draw_ctx.session(); - if shape_def.markers == Markers::Yes { marker_start_node = acquire_marker(session, acquired_nodes, &values.marker_start().0); diff --git a/src/structure.rs b/src/structure.rs index dac71a52c..8fb48c1be 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -368,6 +368,7 @@ impl Draw for Use { values.color().0, cascaded.context_fill.clone(), cascaded.context_stroke.clone(), + draw_ctx.session(), ); let fill_paint = values.fill().0.resolve( @@ -376,6 +377,7 @@ impl Draw for Use { values.color().0, cascaded.context_fill.clone(), cascaded.context_stroke.clone(), + draw_ctx.session(), ); draw_ctx.draw_from_use_node( diff --git a/src/text.rs b/src/text.rs index a635b71b2..b4f77e3b1 100644 --- a/src/text.rs +++ b/src/text.rs @@ -21,6 +21,7 @@ use crate::properties::{ TextAnchor, TextRendering, UnicodeBidi, WritingMode, XmlLang, XmlSpace, }; use crate::rect::Rect; +use crate::session::Session; use crate::space::{xml_space_normalize, NormalizeDefault, XmlSpaceNormalize}; use crate::transform::Transform; use crate::xml::Attributes; @@ -38,6 +39,9 @@ struct LayoutContext { /// For normalizing lengths. view_params: ViewParams, + + /// Session metadata for the document + session: Session, } /// An absolutely-positioned array of `Span`s @@ -451,6 +455,7 @@ impl PositionedSpan { self.values.color().0, None, None, + &layout_context.session, ); let fill_paint = self.values.fill().0.resolve( @@ -459,6 +464,7 @@ impl PositionedSpan { self.values.color().0, None, None, + &layout_context.session, ); let paint_order = self.values.paint_order(); @@ -556,7 +562,14 @@ fn children_to_chunks( Element::TRef(ref tref) => { let cascaded = CascadedValues::clone_with_node(cascaded, &child); - tref.to_chunks(&child, acquired_nodes, &cascaded, chunks, depth + 1); + tref.to_chunks( + &child, + acquired_nodes, + &cascaded, + chunks, + depth + 1, + layout_context, + ); } _ => (), @@ -763,6 +776,7 @@ impl Draw for Text { transform: *transform, font_options: dc.get_font_options(), view_params: dc.get_view_params(), + session: dc.session().clone(), }; let mut x = self.x.to_user(¶ms); @@ -859,6 +873,7 @@ impl TRef { cascaded: &CascadedValues<'_>, chunks: &mut Vec, depth: usize, + layout_context: &LayoutContext, ) { if self.link.is_none() { return; @@ -875,7 +890,8 @@ impl TRef { let c = acquired.get(); extract_chars_children_to_chunks_recursively(chunks, c, Rc::new(values.clone()), depth); } else { - rsvg_log!( + rsvg_log_session!( + layout_context.session, "element {} references a nonexistent text source \"{}\"", node, link, -- GitLab From aaaa42debea82309090cf493f7b61b4f5fe7c7c8 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 19:22:04 -0500 Subject: [PATCH 22/44] api.rs: mark the SvgHandle._session as an unused field It's not used yet, but it will be when we start logging API calls. Part-of: --- src/api.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api.rs b/src/api.rs index 990ee7443..02b123599 100644 --- a/src/api.rs +++ b/src/api.rs @@ -218,7 +218,7 @@ impl Loader { stream.as_ref(), cancellable.map(|c| c.as_ref()), )?, - session: self.session, + _session: self.session, }) } } @@ -232,7 +232,7 @@ fn url_from_file(file: &gio::File) -> Result { /// You can create this from one of the `read` methods in /// [`Loader`]. pub struct SvgHandle { - session: Session, + _session: Session, pub(crate) handle: Handle, } -- GitLab From 99c0c091d9e137fe939d774b9c15ac36b0959946 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 19:23:33 -0500 Subject: [PATCH 23/44] paint_server.rs: Use rsvg_log_session! in a couple of places Part-of: --- src/paint_server.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/paint_server.rs b/src/paint_server.rs index bc699dae3..822fd2334 100644 --- a/src/paint_server.rs +++ b/src/paint_server.rs @@ -181,7 +181,8 @@ impl PaintServer { // later in the drawing code, so it should be fine to translate this // condition to that for an invalid paint server. Some(color) => { - rsvg_log!( + rsvg_log_session!( + session, "could not resolve paint server \"{}\", using alternate color", iri ); @@ -190,7 +191,8 @@ impl PaintServer { } None => { - rsvg_log!( + rsvg_log_session!( + session, "could not resolve paint server \"{}\", no alternate color specified", iri ); -- GitLab From 5341853330ccab1438b32b59edaf856dfe81a1b2 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Mon, 22 Aug 2022 20:52:11 -0500 Subject: [PATCH 24/44] CairoContextState: suppress a warning until we use this for real outside the tests Part-of: --- src/log.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/log.rs b/src/log.rs index 4505e9737..d72924fb7 100644 --- a/src/log.rs +++ b/src/log.rs @@ -45,6 +45,7 @@ struct CairoContextState { } impl CairoContextState { + #[cfg(test)] fn new(cr: &cairo::Context) -> Self { let surface_type = cr.target().type_(); let matrix = cr.matrix(); -- GitLab From 5300bb66d4901ff50b41432c7b5a8ce967613687 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 12:24:21 -0500 Subject: [PATCH 25/44] layout.rs: Use a session for logging This involves passing a Session down to StackingContext::new() Part-of: --- src/drawing_ctx.rs | 20 ++++++++++++++++---- src/image.rs | 8 +++++++- src/layout.rs | 11 ++++++++--- src/marker.rs | 3 ++- src/shapes.rs | 9 +++++++-- src/structure.rs | 25 ++++++++++++++++++++++--- src/text.rs | 8 +++++++- 7 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs index 9193d1429..0342cfb51 100644 --- a/src/drawing_ctx.rs +++ b/src/drawing_ctx.rs @@ -654,8 +654,13 @@ impl DrawingCtx { let mut mask_draw_ctx = self.nested(mask_cr); - let stacking_ctx = - StackingContext::new(acquired_nodes, &mask_element, Transform::identity(), values); + let stacking_ctx = StackingContext::new( + self.session(), + acquired_nodes, + &mask_element, + Transform::identity(), + values, + ); let res = mask_draw_ctx.with_discrete_layer( &stacking_ctx, @@ -1111,6 +1116,7 @@ impl DrawingCtx { let elt = pattern_node.borrow_element(); let stacking_ctx = StackingContext::new( + self.session(), acquired_nodes, &elt, Transform::identity(), @@ -1694,8 +1700,13 @@ impl DrawingCtx { None }; - let stacking_ctx = - StackingContext::new(acquired_nodes, &use_element, Transform::identity(), values); + let stacking_ctx = StackingContext::new( + self.session(), + acquired_nodes, + &use_element, + Transform::identity(), + values, + ); self.with_discrete_layer( &stacking_ctx, @@ -1728,6 +1739,7 @@ impl DrawingCtx { // otherwise the referenced node is not a ; process it generically let stacking_ctx = StackingContext::new( + self.session(), acquired_nodes, &use_element, Transform::new_translate(use_rect.x0, use_rect.y0), diff --git a/src/image.rs b/src/image.rs index b825ef342..a8c3ab93a 100644 --- a/src/image.rs +++ b/src/image.rs @@ -97,7 +97,13 @@ impl Draw for Image { }; let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.draw_image(&image, &stacking_ctx, acquired_nodes, values, clipping) } diff --git a/src/layout.rs b/src/layout.rs index f68eb1cf5..c67164a45 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -21,6 +21,7 @@ use crate::properties::{ TextDecoration, TextRendering, UnicodeBidi, XmlLang, }; use crate::rect::Rect; +use crate::session::Session; use crate::surface_utils::shared_surface::SharedImageSurface; use crate::transform::Transform; use crate::unit_interval::UnitInterval; @@ -137,6 +138,7 @@ pub struct FontProperties { impl StackingContext { pub fn new( + session: &Session, acquired_nodes: &mut AcquiredNodes<'_>, element: &Element, transform: Transform, @@ -189,7 +191,8 @@ impl StackingContext { Element::Mask(_) => Some(node.clone()), _ => { - rsvg_log!( + rsvg_log_session!( + session, "element {} references \"{}\" which is not a mask", element, mask_id @@ -199,7 +202,8 @@ impl StackingContext { } } } else { - rsvg_log!( + rsvg_log_session!( + session, "element {} references nonexistent mask \"{}\"", element, mask_id @@ -227,13 +231,14 @@ impl StackingContext { } pub fn new_with_link( + session: &Session, acquired_nodes: &mut AcquiredNodes<'_>, element: &Element, transform: Transform, values: &ComputedValues, link_target: Option, ) -> StackingContext { - let mut ctx = Self::new(acquired_nodes, element, transform, values); + let mut ctx = Self::new(session, acquired_nodes, element, transform, values); ctx.link_target = link_target; ctx } diff --git a/src/marker.rs b/src/marker.rs index 5acab183f..87c0135f4 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -184,7 +184,8 @@ impl Marker { }; let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, transform, values); + let stacking_ctx = + StackingContext::new(draw_ctx.session(), acquired_nodes, &elt, transform, values); draw_ctx.with_discrete_layer( &stacking_ctx, diff --git a/src/shapes.rs b/src/shapes.rs index b617da9e8..d7a161b4c 100644 --- a/src/shapes.rs +++ b/src/shapes.rs @@ -138,8 +138,13 @@ macro_rules! impl_draw { }; let elt = node.borrow_element(); - let stacking_ctx = - StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.draw_shape( &view_params, diff --git a/src/structure.rs b/src/structure.rs index 8fb48c1be..da0457284 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -36,7 +36,13 @@ impl Draw for Group { let values = cascaded.get(); let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.with_discrete_layer( &stacking_ctx, @@ -77,7 +83,13 @@ impl Draw for Switch { let values = cascaded.get(); let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.with_discrete_layer( &stacking_ctx, @@ -279,7 +291,13 @@ impl Draw for Svg { let values = cascaded.get(); let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.with_discrete_layer( &stacking_ctx, @@ -575,6 +593,7 @@ impl Draw for Link { }; let stacking_ctx = StackingContext::new_with_link( + draw_ctx.session(), acquired_nodes, &elt, values.transform(), diff --git a/src/text.rs b/src/text.rs index b4f77e3b1..dc4f65e72 100644 --- a/src/text.rs +++ b/src/text.rs @@ -762,7 +762,13 @@ impl Draw for Text { let elt = node.borrow_element(); - let stacking_ctx = StackingContext::new(acquired_nodes, &elt, values.transform(), values); + let stacking_ctx = StackingContext::new( + draw_ctx.session(), + acquired_nodes, + &elt, + values.transform(), + values, + ); draw_ctx.with_discrete_layer( &stacking_ctx, -- GitLab From 63ee11c7863f19e4a0e6d990d163e7e6ae39d762 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 12:29:37 -0500 Subject: [PATCH 26/44] filter.rs: Use the DrawingCtx's session for logging Part-of: --- src/filter.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/filter.rs b/src/filter.rs index 98552aea8..2be886b3e 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -125,10 +125,13 @@ fn filter_spec_from_filter_node( node_id: &NodeId, node_being_filtered_name: &str, ) -> Result { + let session = draw_ctx.session().clone(); + acquired_nodes .acquire(node_id) .map_err(|e| { - rsvg_log!( + rsvg_log_session!( + session, "element {} will not be filtered with \"{}\": {}", node_being_filtered_name, node_id, @@ -143,7 +146,8 @@ fn filter_spec_from_filter_node( match *element { Element::Filter(_) => { if element.is_in_error() { - rsvg_log!( + rsvg_log_session!( + session, "element {} will not be filtered since its filter \"{}\" is in error", node_being_filtered_name, node_id, @@ -155,7 +159,8 @@ fn filter_spec_from_filter_node( } _ => { - rsvg_log!( + rsvg_log_session!( + session, "element {} will not be filtered since \"{}\" is not a filter", node_being_filtered_name, node_id, -- GitLab From 8919cf8afdca227eecf0084d44b877be0f075284 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 12:31:08 -0500 Subject: [PATCH 27/44] image.rs: Use a session for logging Part-of: --- src/image.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/image.rs b/src/image.rs index a8c3ab93a..b674a51e9 100644 --- a/src/image.rs +++ b/src/image.rs @@ -58,7 +58,12 @@ impl Draw for Image { Some(ref url) => match acquired_nodes.lookup_image(url) { Ok(surf) => surf, Err(e) => { - rsvg_log!("could not load image \"{}\": {}", url, e); + rsvg_log_session!( + draw_ctx.session(), + "could not load image \"{}\": {}", + url, + e + ); return Ok(draw_ctx.empty_bbox()); } }, -- GitLab From 7e9b0df1af2f3a88d2bd13fabc581e275384d51c Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 12:33:18 -0500 Subject: [PATCH 28/44] filters/mod.rs: Use a session for logging Part-of: --- src/filters/mod.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/filters/mod.rs b/src/filters/mod.rs index 0b3136cdf..2f9f13aec 100644 --- a/src/filters/mod.rs +++ b/src/filters/mod.rs @@ -246,6 +246,8 @@ pub fn extract_filter_from_filter_node( acquired_nodes: &mut AcquiredNodes<'_>, draw_ctx: &DrawingCtx, ) -> Result { + let session = draw_ctx.session().clone(); + let filter_node = &*filter_node; assert!(is_element_of_type!(filter_node, Filter)); @@ -270,7 +272,11 @@ pub fn extract_filter_from_filter_node( let in_error = c.borrow_element().is_in_error(); if in_error { - rsvg_log!("(ignoring filter primitive {} because it is in error)", c); + rsvg_log_session!( + session, + "(ignoring filter primitive {} because it is in error)", + c + ); } !in_error @@ -292,7 +298,8 @@ pub fn extract_filter_from_filter_node( effect .resolve(acquired_nodes, &primitive_node) .map_err(|e| { - rsvg_log!( + rsvg_log_session!( + session, "(filter primitive {} returned an error: {})", primitive_name, e @@ -320,6 +327,8 @@ pub fn render( transform: Transform, node_bbox: BoundingBox, ) -> Result { + let session = draw_ctx.session().clone(); + FilterContext::new( &filter.user_space_filter, stroke_paint_source, @@ -335,7 +344,8 @@ pub fn render( match render_primitive(user_space_primitive, &filter_ctx, acquired_nodes, draw_ctx) { Ok(output) => { let elapsed = start.elapsed(); - rsvg_log!( + rsvg_log_session!( + session, "(rendered filter primitive {} in\n {} seconds)", user_space_primitive.params.name(), elapsed.as_secs() as f64 + f64::from(elapsed.subsec_nanos()) / 1e9 @@ -348,7 +358,8 @@ pub fn render( } Err(err) => { - rsvg_log!( + rsvg_log_session!( + session, "(filter primitive {} returned an error: {})", user_space_primitive.params.name(), err -- GitLab From f6be4ee89e1f9ff463cde9da9dec5d74274d92da Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 13:38:32 -0500 Subject: [PATCH 29/44] impl Default for Session Clippy had suggested this, but we'll need it to make things easy in c_api. Part-of: --- src/api.rs | 2 +- src/session.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/api.rs b/src/api.rs index 02b123599..4c97955b4 100644 --- a/src/api.rs +++ b/src/api.rs @@ -59,7 +59,7 @@ impl Loader { /// .unwrap(); /// ``` pub fn new() -> Self { - Self::new_with_session(Session::new()) + Self::new_with_session(Session::default()) } /// Creates a `Loader` from a pre-created [`Session`]. diff --git a/src/session.rs b/src/session.rs index 87385aa8a..1f2b998b0 100644 --- a/src/session.rs +++ b/src/session.rs @@ -18,15 +18,17 @@ struct SessionInner { log_enabled: bool, } -impl Session { - pub fn new() -> Self { +impl Default for Session { + fn default() -> Self { Self { inner: Arc::new(SessionInner { log_enabled: log::log_enabled(), }), } } +} +impl Session { #[cfg(test)] pub fn new_for_test_suite() -> Self { Self { -- GitLab From 0c0bbf572bb9ff5f5178dcbedf6a6396f6d2dab2 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 13:53:10 -0500 Subject: [PATCH 30/44] c_api/handle.rs: Keep a Session for the CHandle and log to it Part-of: --- src/c_api/handle.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/c_api/handle.rs b/src/c_api/handle.rs index 3427273c7..a47bffd89 100644 --- a/src/c_api/handle.rs +++ b/src/c_api/handle.rs @@ -42,7 +42,8 @@ use crate::api::{self, CairoRenderer, IntrinsicDimensions, Loader, LoadingError, use crate::{ length::RsvgLength, - rsvg_log, + rsvg_log_session, + session::Session, surface_utils::shared_surface::{SharedImageSurface, SurfaceType}, }; @@ -297,6 +298,7 @@ mod imp { pub struct CHandle { pub(super) inner: RefCell, pub(super) load_state: RefCell, + pub(super) session: Session, } #[derive(Default)] @@ -559,6 +561,7 @@ impl CairoRectangleExt for cairo::Rectangle { impl CHandle { fn set_base_url(&self, url: &str) { let imp = self.imp(); + let session = &imp.session; let state = imp.load_state.borrow(); match *state { @@ -573,13 +576,14 @@ impl CHandle { match Url::parse(url) { Ok(u) => { - rsvg_log!("setting base_uri to \"{}\"", u.as_str()); + rsvg_log_session!(session, "setting base_uri to \"{}\"", u.as_str()); let mut inner = imp.inner.borrow_mut(); inner.base_url.set(u); } Err(e) => { - rsvg_log!( + rsvg_log_session!( + session, "not setting base_uri to \"{}\" since it is invalid: {}", url, e @@ -760,9 +764,11 @@ impl CHandle { } fn make_loader(&self) -> Loader { - let inner = self.imp().inner.borrow(); + let imp = self.imp(); + let inner = imp.inner.borrow(); + let session = imp.session.clone(); - Loader::new() + Loader::new_with_session(session) .with_unlimited_size(inner.load_flags.unlimited_size) .keep_image_data(inner.load_flags.keep_image_data) } @@ -1326,7 +1332,8 @@ pub unsafe extern "C" fn rsvg_handle_get_pixbuf( match rhandle.get_pixbuf_sub(None) { Ok(pixbuf) => pixbuf.to_glib_full(), Err(e) => { - rsvg_log!("could not render: {}", e); + let session = &rhandle.imp().session; + rsvg_log_session!(session, "could not render: {}", e); ptr::null_mut() } } @@ -1349,7 +1356,8 @@ pub unsafe extern "C" fn rsvg_handle_get_pixbuf_sub( match rhandle.get_pixbuf_sub(id.as_deref()) { Ok(pixbuf) => pixbuf.to_glib_full(), Err(e) => { - rsvg_log!("could not render: {}", e); + let session = &rhandle.imp().session; + rsvg_log_session!(session, "could not render: {}", e); ptr::null_mut() } } @@ -1387,7 +1395,8 @@ pub unsafe extern "C" fn rsvg_handle_get_dimensions_sub( } Err(e) => { - rsvg_log!("could not get dimensions: {}", e); + let session = &rhandle.imp().session; + rsvg_log_session!(session, "could not get dimensions: {}", e); *dimension_data = RsvgDimensionData::empty(); false.into_glib() } @@ -1423,7 +1432,8 @@ pub unsafe extern "C" fn rsvg_handle_get_position_sub( p.x = 0; p.y = 0; - rsvg_log!("could not get position: {}", e); + let session = &rhandle.imp().session; + rsvg_log_session!(session, "could not get position: {}", e); false.into_glib() } } -- GitLab From ab21884ffaaaa7fcfc286d89e9841883a100e62a Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 14:42:02 -0500 Subject: [PATCH 31/44] c_api: Pass a session to set_gerror Part-of: --- src/c_api/handle.rs | 73 +++++++++++++++++++++++++++++---------- src/c_api/pixbuf_utils.rs | 13 ++++--- 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/c_api/handle.rs b/src/c_api/handle.rs index a47bffd89..d68e60cdc 100644 --- a/src/c_api/handle.rs +++ b/src/c_api/handle.rs @@ -1173,18 +1173,23 @@ pub unsafe extern "C" fn rsvg_handle_internal_set_testing( trait IntoGError { type GlibResult; - fn into_gerror(self, error: *mut *mut glib::ffi::GError) -> Self::GlibResult; + fn into_gerror(self, session: &Session, error: *mut *mut glib::ffi::GError) + -> Self::GlibResult; } impl IntoGError for Result<(), E> { type GlibResult = glib::ffi::gboolean; - fn into_gerror(self, error: *mut *mut glib::ffi::GError) -> Self::GlibResult { + fn into_gerror( + self, + session: &Session, + error: *mut *mut glib::ffi::GError, + ) -> Self::GlibResult { match self { Ok(()) => true.into_glib(), Err(e) => { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(session, error, 0, &format!("{}", e)); false.into_glib() } } @@ -1208,13 +1213,14 @@ pub unsafe extern "C" fn rsvg_handle_read_stream_sync( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); let stream = gio::InputStream::from_glib_none(stream); let cancellable: Option = from_glib_none(cancellable); rhandle .read_stream_sync(&stream, cancellable.as_ref()) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1252,8 +1258,9 @@ pub unsafe extern "C" fn rsvg_handle_close( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); - rhandle.close().into_gerror(error) + rhandle.close().into_gerror(&session, error) } #[no_mangle] @@ -1290,10 +1297,11 @@ pub unsafe extern "C" fn rsvg_handle_render_cairo( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); rhandle .render_cairo_sub(cr, None) - .into_gerror(ptr::null_mut()) + .into_gerror(&session, ptr::null_mut()) } #[no_mangle] @@ -1310,11 +1318,13 @@ pub unsafe extern "C" fn rsvg_handle_render_cairo_sub( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); + let id: Option = from_glib_none(id); rhandle .render_cairo_sub(cr, id.as_deref()) - .into_gerror(ptr::null_mut()) + .into_gerror(&session, ptr::null_mut()) } #[no_mangle] @@ -1470,7 +1480,12 @@ pub unsafe extern "C" fn rsvg_handle_new_from_file( Ok(p) => p.get_gfile(), Err(s) => { - set_gerror(error, 0, &s); + // Here we don't have a handle created yet, so it's fine to create a session + // to log the error message. We'll need to change this when we start logging + // API calls, so that we can log the call to rsvg_handle_new_from_file() and + // then pass *that* session to rsvg_handle_new_from_gfile_sync() below. + let session = Session::default(); + set_gerror(&session, error, 0, &s); return ptr::null_mut(); } }; @@ -1496,6 +1511,7 @@ pub unsafe extern "C" fn rsvg_handle_new_from_gfile_sync( let raw_handle = rsvg_handle_new_with_flags(flags); let rhandle = get_rust_handle(raw_handle); + let session = rhandle.imp().session.clone(); let file = gio::File::from_glib_none(file); rhandle.set_base_gfile(&file); @@ -1511,7 +1527,7 @@ pub unsafe extern "C" fn rsvg_handle_new_from_gfile_sync( Ok(()) => raw_handle, Err(e) => { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(&session, error, 0, &format!("{}", e)); gobject_ffi::g_object_unref(raw_handle as *mut _); ptr::null_mut() } @@ -1538,6 +1554,7 @@ pub unsafe extern "C" fn rsvg_handle_new_from_stream_sync( let raw_handle = rsvg_handle_new_with_flags(flags); let rhandle = get_rust_handle(raw_handle); + let session = rhandle.imp().session.clone(); let base_file: Option = from_glib_none(base_file); if let Some(base_file) = base_file { @@ -1551,7 +1568,7 @@ pub unsafe extern "C" fn rsvg_handle_new_from_stream_sync( Ok(()) => raw_handle, Err(e) => { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(&session, error, 0, &format!("{}", e)); gobject_ffi::g_object_unref(raw_handle as *mut _); ptr::null_mut() } @@ -1640,6 +1657,7 @@ pub unsafe extern "C" fn rsvg_handle_set_stylesheet( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); let css = match (css, css_len) { (p, 0) if p.is_null() => "", @@ -1648,14 +1666,19 @@ pub unsafe extern "C" fn rsvg_handle_set_stylesheet( match str::from_utf8(s) { Ok(s) => s, Err(e) => { - set_gerror(error, 0, &format!("CSS is not valid UTF-8: {}", e)); + set_gerror( + &session, + error, + 0, + &format!("CSS is not valid UTF-8: {}", e), + ); return false.into_glib(); } } } }; - rhandle.set_stylesheet(css).into_gerror(error) + rhandle.set_stylesheet(css).into_gerror(&session, error) } #[no_mangle] @@ -1737,10 +1760,11 @@ pub unsafe extern "C" fn rsvg_handle_render_document( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); rhandle .render_document(cr, &(*viewport).into()) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1761,6 +1785,7 @@ pub unsafe extern "C" fn rsvg_handle_get_geometry_for_layer( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); let id: Option = from_glib_none(id); @@ -1775,7 +1800,7 @@ pub unsafe extern "C" fn rsvg_handle_get_geometry_for_layer( *out_logical_rect = logical_rect; } }) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1796,11 +1821,13 @@ pub unsafe extern "C" fn rsvg_handle_render_layer( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); + let id: Option = from_glib_none(id); rhandle .render_layer(cr, id.as_deref(), &(*viewport).into()) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1819,6 +1846,7 @@ pub unsafe extern "C" fn rsvg_handle_get_geometry_for_element( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); let id: Option = from_glib_none(id); @@ -1833,7 +1861,7 @@ pub unsafe extern "C" fn rsvg_handle_get_geometry_for_element( *out_logical_rect = logical_rect; } }) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1854,11 +1882,13 @@ pub unsafe extern "C" fn rsvg_handle_render_element( } let rhandle = get_rust_handle(handle); + let session = rhandle.imp().session.clone(); + let id: Option = from_glib_none(id); rhandle .render_element(cr, id.as_deref(), &(*element_viewport).into()) - .into_gerror(error) + .into_gerror(&session, error) } #[no_mangle] @@ -1995,7 +2025,12 @@ fn check_cairo_context(cr: *mut cairo::ffi::cairo_t) -> Result Result { @@ -155,10 +156,12 @@ unsafe fn pixbuf_from_file_with_size_mode( ) -> *mut gdk_pixbuf::ffi::GdkPixbuf { let path = PathBuf::from_glib_none(filename); - let handle = match Loader::new().read_path(path) { + let session = Session::default(); + + let handle = match Loader::new_with_session(session.clone()).read_path(path) { Ok(handle) => handle, Err(e) => { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(&session, error, 0, &format!("{}", e)); return ptr::null_mut(); } }; @@ -169,7 +172,7 @@ unsafe fn pixbuf_from_file_with_size_mode( let (document_width, document_height) = match renderer.legacy_document_size() { Ok(dim) => dim, Err(e) => { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(&session, error, 0, &format!("{}", e)); return ptr::null_mut(); } }; @@ -186,7 +189,7 @@ unsafe fn pixbuf_from_file_with_size_mode( ) .map(|pixbuf| pixbuf.to_glib_full()) .unwrap_or_else(|e| { - set_gerror(error, 0, &format!("{}", e)); + set_gerror(&session, error, 0, &format!("{}", e)); ptr::null_mut() }) } -- GitLab From ad2b1d3c62674204a33732b35d3de183171ebca7 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 17:26:51 -0500 Subject: [PATCH 32/44] css.rs: Use a session for logging Part-of: --- src/css.rs | 46 ++++++++++++++++++++++++++++++++-------------- src/document.rs | 21 ++++++++++++++------- src/handle.rs | 2 +- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/css.rs b/src/css.rs index e9b80bbda..2f646674e 100644 --- a/src/css.rs +++ b/src/css.rs @@ -93,6 +93,7 @@ use crate::error::*; use crate::io::{self, BinaryData}; use crate::node::{Node, NodeBorrow, NodeCascade}; use crate::properties::{parse_value, ComputedValues, ParseAs, ParsedProperty}; +use crate::session::Session; use crate::url_resolver::UrlResolver; /// A parsed CSS declaration @@ -147,9 +148,10 @@ impl<'i> AtRuleParser<'i> for DeclParser { type Error = ValueErrorKind; } -/// Dummy struct to implement cssparser::QualifiedRuleParser and -/// cssparser::AtRuleParser -pub struct RuleParser; +/// Struct to implement cssparser::QualifiedRuleParser and cssparser::AtRuleParser +pub struct RuleParser { + session: Session, +} /// Errors from the CSS parsing process #[derive(Debug)] @@ -283,7 +285,7 @@ impl<'i> QualifiedRuleParser<'i> for RuleParser { .filter_map(|r| match r { Ok(decl) => Some(decl), Err(e) => { - rsvg_log!("Invalid declaration; ignoring: {:?}", e); + rsvg_log_session!(self.session, "Invalid declaration; ignoring: {:?}", e); None } }) @@ -804,9 +806,10 @@ impl Stylesheet { buf: &str, url_resolver: &UrlResolver, origin: Origin, + session: Session, ) -> Result { let mut stylesheet = Stylesheet::new(origin); - stylesheet.parse(buf, url_resolver)?; + stylesheet.parse(buf, url_resolver, session)?; Ok(stylesheet) } @@ -814,9 +817,10 @@ impl Stylesheet { href: &str, url_resolver: &UrlResolver, origin: Origin, + session: Session, ) -> Result { let mut stylesheet = Stylesheet::new(origin); - stylesheet.load(href, url_resolver)?; + stylesheet.load(href, url_resolver, session)?; Ok(stylesheet) } @@ -824,22 +828,30 @@ impl Stylesheet { /// /// The `base_url` is required for `@import` rules, so that librsvg /// can determine if the requested path is allowed. - pub fn parse(&mut self, buf: &str, url_resolver: &UrlResolver) -> Result<(), LoadingError> { + pub fn parse( + &mut self, + buf: &str, + url_resolver: &UrlResolver, + session: Session, + ) -> Result<(), LoadingError> { let mut input = ParserInput::new(buf); let mut parser = Parser::new(&mut input); + let rule_parser = RuleParser { + session: session.clone(), + }; - RuleListParser::new_for_stylesheet(&mut parser, RuleParser) + RuleListParser::new_for_stylesheet(&mut parser, rule_parser) .filter_map(|r| match r { Ok(rule) => Some(rule), Err(e) => { - rsvg_log!("Invalid rule; ignoring: {:?}", e); + rsvg_log_session!(session, "Invalid rule; ignoring: {:?}", e); None } }) .for_each(|rule| match rule { Rule::AtRule(AtRule::Import(url)) => { // ignore invalid imports - let _ = self.load(&url, url_resolver); + let _ = self.load(&url, url_resolver, session.clone()); } Rule::QualifiedRule(qr) => self.qualified_rules.push(qr), }); @@ -848,7 +860,12 @@ impl Stylesheet { } /// Parses a stylesheet referenced by an URL - fn load(&mut self, href: &str, url_resolver: &UrlResolver) -> Result<(), LoadingError> { + fn load( + &mut self, + href: &str, + url_resolver: &UrlResolver, + session: Session, + ) -> Result<(), LoadingError> { let aurl = url_resolver .resolve_href(href) .map_err(|_| LoadingError::BadUrl)?; @@ -864,20 +881,21 @@ impl Stylesheet { if is_text_css(&mime_type) { Ok(bytes) } else { - rsvg_log!("\"{}\" is not of type text/css; ignoring", aurl); + rsvg_log_session!(session, "\"{}\" is not of type text/css; ignoring", aurl); Err(LoadingError::BadCss) } }) .and_then(|bytes| { String::from_utf8(bytes).map_err(|_| { - rsvg_log!( + rsvg_log_session!( + session, "\"{}\" does not contain valid UTF-8 CSS data; ignoring", aurl ); LoadingError::BadCss }) }) - .and_then(|utf8| self.parse(&utf8, &UrlResolver::new(Some((*aurl).clone())))) + .and_then(|utf8| self.parse(&utf8, &UrlResolver::new(Some((*aurl).clone())), session)) } /// Appends the style declarations that match a specified node to a given vector diff --git a/src/document.rs b/src/document.rs index fb9071ef2..1a7650c11 100644 --- a/src/document.rs +++ b/src/document.rs @@ -28,8 +28,9 @@ static UA_STYLESHEETS: Lazy> = Lazy::new(|| { include_str!("ua.css"), &UrlResolver::new(None), Origin::UserAgent, + Session::default(), ) - .unwrap()] + .expect("could not parse user agent stylesheet for librsvg, there's a bug!")] }); /// Identifier of a node @@ -523,9 +524,12 @@ impl DocumentBuilder { } // FIXME: handle CSS errors - if let Ok(stylesheet) = - Stylesheet::from_href(href, &self.load_options.url_resolver, Origin::Author) - { + if let Ok(stylesheet) = Stylesheet::from_href( + href, + &self.load_options.url_resolver, + Origin::Author, + self.session.clone(), + ) { self.stylesheets.push(stylesheet); } @@ -560,9 +564,12 @@ impl DocumentBuilder { pub fn append_stylesheet_from_text(&mut self, text: &str) { // FIXME: handle CSS errors - if let Ok(stylesheet) = - Stylesheet::from_data(text, &self.load_options.url_resolver, Origin::Author) - { + if let Ok(stylesheet) = Stylesheet::from_data( + text, + &self.load_options.url_resolver, + Origin::Author, + self.session.clone(), + ) { self.stylesheets.push(stylesheet); } } diff --git a/src/handle.rs b/src/handle.rs index 8dec2a880..6062d3f13 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -377,7 +377,7 @@ impl Handle { pub fn set_stylesheet(&mut self, css: &str) -> Result<(), LoadingError> { let mut stylesheet = Stylesheet::new(Origin::User); - stylesheet.parse(css, &UrlResolver::new(None))?; + stylesheet.parse(css, &UrlResolver::new(None), self.session.clone())?; self.document.cascade(&[stylesheet]); Ok(()) } -- GitLab From c0a336b8a3121033c0ddf59444a8ab7a26f95dff Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 17:34:46 -0500 Subject: [PATCH 33/44] Pass a Session to the node creation code Part-of: --- src/document.rs | 2 +- src/element.rs | 22 +++++++++++++++------- src/gradient.rs | 5 +++++ src/node.rs | 5 +++-- src/pattern.rs | 1 + src/properties.rs | 9 ++++++--- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/document.rs b/src/document.rs index 1a7650c11..0af3d9e5c 100644 --- a/src/document.rs +++ b/src/document.rs @@ -542,7 +542,7 @@ impl DocumentBuilder { attrs: Attributes, parent: Option, ) -> Node { - let node = Node::new(NodeData::new_element(name, attrs)); + let node = Node::new(NodeData::new_element(&self.session, name, attrs)); if let Some(id) = node.borrow_element().get_id() { // This is so we don't overwrite an existing id diff --git a/src/element.rs b/src/element.rs index ffa47fb08..2926680fe 100644 --- a/src/element.rs +++ b/src/element.rs @@ -38,6 +38,7 @@ use crate::marker::Marker; use crate::node::*; use crate::pattern::Pattern; use crate::properties::{ComputedValues, SpecifiedValues}; +use crate::session::Session; use crate::shapes::{Circle, Ellipse, Line, Path, Polygon, Polyline, Rect}; use crate::structure::{ClipPath, Group, Link, Mask, NonRendering, Svg, Switch, Symbol, Use}; use crate::style::Style; @@ -107,6 +108,7 @@ pub struct ElementInner { impl ElementInner { fn new( + session: &Session, element_name: QualName, attributes: Attributes, result: Result<(), ElementError>, @@ -127,7 +129,7 @@ impl ElementInner { let mut set_attributes = || -> Result<(), ElementError> { e.set_conditional_processing_attributes()?; - e.set_presentation_attributes()?; + e.set_presentation_attributes(session)?; Ok(()) }; @@ -215,9 +217,9 @@ impl ElementInner { /// Hands the `attrs` to the node's state, to apply the presentation attributes. #[allow(clippy::unnecessary_wraps)] - fn set_presentation_attributes(&mut self) -> Result<(), ElementError> { + fn set_presentation_attributes(&mut self, session: &Session) -> Result<(), ElementError> { self.specified_values - .parse_presentation_attributes(&self.attributes) + .parse_presentation_attributes(session, &self.attributes) } // Applies a style declaration to the node's specified_values @@ -449,7 +451,7 @@ impl Element { /// /// This operation does not fail. Unknown element names simply produce a [`NonRendering`] /// element. - pub fn new(name: &QualName, mut attrs: Attributes) -> Element { + pub fn new(session: &Session, name: &QualName, mut attrs: Attributes) -> Element { let (create_fn, flags): (ElementCreateFn, ElementCreateFlags) = if name.ns == ns!(svg) { match ELEMENT_CREATORS.get(name.local.as_ref()) { // hack in the SVG namespace for supported element names @@ -470,7 +472,7 @@ impl Element { // sizes::print_sizes(); - create_fn(name, attrs) + create_fn(session, name, attrs) } pub fn element_name(&self) -> &QualName { @@ -589,12 +591,17 @@ impl fmt::Display for Element { macro_rules! e { ($name:ident, $element_type:ident) => { - pub fn $name(element_name: &QualName, attributes: Attributes) -> Element { + pub fn $name( + session: &Session, + element_name: &QualName, + attributes: Attributes, + ) -> Element { let mut element_impl = <$element_type>::default(); let result = element_impl.set_attributes(&attributes); let element = Element::$element_type(Box::new(ElementInner::new( + session, element_name.clone(), attributes, result, @@ -679,7 +686,8 @@ mod creators { use creators::*; -type ElementCreateFn = fn(element_name: &QualName, attributes: Attributes) -> Element; +type ElementCreateFn = + fn(session: &Session, element_name: &QualName, attributes: Attributes) -> Element; #[derive(Copy, Clone, PartialEq)] enum ElementCreateFlags { diff --git a/src/gradient.rs b/src/gradient.rs index ac914e763..b1e6813e7 100644 --- a/src/gradient.rs +++ b/src/gradient.rs @@ -716,6 +716,7 @@ impl ResolvedGradient { mod tests { use super::*; use crate::node::{Node, NodeData}; + use crate::session::Session; use markup5ever::{namespace_url, ns, QualName}; #[test] @@ -734,7 +735,10 @@ mod tests { #[test] fn gradient_resolved_from_defaults_is_really_resolved() { + let session = Session::default(); + let node = Node::new(NodeData::new_element( + &session, &QualName::new(None, ns!(svg), local_name!("linearGradient")), Attributes::new(), )); @@ -745,6 +749,7 @@ mod tests { assert!(gradient.is_resolved()); let node = Node::new(NodeData::new_element( + &session, &QualName::new(None, ns!(svg), local_name!("radialGradient")), Attributes::new(), )); diff --git a/src/node.rs b/src/node.rs index 5d99ec9a6..52f655a5a 100644 --- a/src/node.rs +++ b/src/node.rs @@ -18,6 +18,7 @@ use crate::element::*; use crate::error::*; use crate::paint_server::PaintSource; use crate::properties::ComputedValues; +use crate::session::Session; use crate::text::Chars; use crate::xml::Attributes; @@ -65,8 +66,8 @@ pub enum NodeData { } impl NodeData { - pub fn new_element(name: &QualName, attrs: Attributes) -> NodeData { - NodeData::Element(Element::new(name, attrs)) + pub fn new_element(session: &Session, name: &QualName, attrs: Attributes) -> NodeData { + NodeData::Element(Element::new(session, name, attrs)) } pub fn new_chars(initial_text: &str) -> NodeData { diff --git a/src/pattern.rs b/src/pattern.rs index e14bb3e29..7152c13e6 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -497,6 +497,7 @@ mod tests { #[test] fn pattern_resolved_from_defaults_is_really_resolved() { let node = Node::new(NodeData::new_element( + &Session::default(), &QualName::new(None, ns!(svg), local_name!("pattern")), Attributes::new(), )); diff --git a/src/properties.rs b/src/properties.rs index 893afaf00..8d4390d9f 100644 --- a/src/properties.rs +++ b/src/properties.rs @@ -26,6 +26,7 @@ use crate::css::{DeclParser, Declaration, Origin}; use crate::error::*; use crate::parsers::{Parse, ParseValue}; use crate::property_macros::Property; +use crate::session::Session; use crate::transform::{Transform, TransformAttribute, TransformProperty}; use crate::xml::Attributes; @@ -805,7 +806,7 @@ impl SpecifiedValues { } } - fn parse_one_presentation_attribute(&mut self, attr: QualName, value: &str) { + fn parse_one_presentation_attribute(&mut self, session: &Session, attr: QualName, value: &str) { let mut input = ParserInput::new(value); let mut parser = Parser::new(&mut input); @@ -814,7 +815,8 @@ impl SpecifiedValues { if parser.expect_exhausted().is_ok() { self.set_parsed_property(&prop); } else { - rsvg_log!( + rsvg_log_session!( + session, "(ignoring invalid presentation attribute {:?}\n value=\"{}\")\n", attr.expanded(), value, @@ -887,6 +889,7 @@ impl SpecifiedValues { pub fn parse_presentation_attributes( &mut self, + session: &Session, attrs: &Attributes, ) -> Result<(), ElementError> { for (attr, value) in attrs.iter() { @@ -918,7 +921,7 @@ impl SpecifiedValues { ))); } - _ => self.parse_one_presentation_attribute(attr, value), + _ => self.parse_one_presentation_attribute(session, attr, value), } } -- GitLab From 03fa7caa2772abc84b979ede060b0c1efaae5632 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 17:41:05 -0500 Subject: [PATCH 34/44] Pass a Session to the cascading machinery Part-of: --- src/css.rs | 3 ++- src/document.rs | 14 ++++++++++---- src/element.rs | 7 ++++--- src/handle.rs | 2 +- src/properties.rs | 17 +++++++++++------ 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/css.rs b/src/css.rs index 2f646674e..5d5d0abd5 100644 --- a/src/css.rs +++ b/src/css.rs @@ -941,6 +941,7 @@ pub fn cascade( ua_stylesheets: &[Stylesheet], author_stylesheets: &[Stylesheet], user_stylesheets: &[Stylesheet], + session: &Session, ) { for mut node in root.descendants().filter(|n| n.is_element()) { let mut matches = Vec::new(); @@ -975,7 +976,7 @@ pub fn cascade( .apply_style_declaration(m.declaration, m.origin); } - node.borrow_element_mut().set_style_attribute(); + node.borrow_element_mut().set_style_attribute(session); } let values = ComputedValues::default(); diff --git a/src/document.rs b/src/document.rs index 0af3d9e5c..21252e407 100644 --- a/src/document.rs +++ b/src/document.rs @@ -166,8 +166,14 @@ impl Document { /// /// This uses the default UserAgent stylesheet, the document's internal stylesheets, /// plus an extra set of stylesheets supplied by the caller. - pub fn cascade(&mut self, extra: &[Stylesheet]) { - css::cascade(&mut self.tree, &UA_STYLESHEETS, &self.stylesheets, extra); + pub fn cascade(&mut self, extra: &[Stylesheet], session: &Session) { + css::cascade( + &mut self.tree, + &UA_STYLESHEETS, + &self.stylesheets, + extra, + session, + ); } } @@ -609,7 +615,7 @@ impl DocumentBuilder { if is_element_of_type!(root, Svg) { let mut document = Document { tree: root, - session, + session: session.clone(), ids, externs: RefCell::new(Resources::new()), images: RefCell::new(Images::new()), @@ -617,7 +623,7 @@ impl DocumentBuilder { stylesheets, }; - document.cascade(&[]); + document.cascade(&[], &session); Ok(document) } else { diff --git a/src/element.rs b/src/element.rs index 2926680fe..7db33608d 100644 --- a/src/element.rs +++ b/src/element.rs @@ -232,7 +232,7 @@ impl ElementInner { } /// Applies CSS styles from the "style" attribute - fn set_style_attribute(&mut self) { + fn set_style_attribute(&mut self, session: &Session) { let style = self .attributes .iter() @@ -244,6 +244,7 @@ impl ElementInner { style, Origin::Author, &mut self.important_styles, + session, ) { self.set_error(e); } @@ -515,8 +516,8 @@ impl Element { call_inner!(self, apply_style_declaration, declaration, origin) } - pub fn set_style_attribute(&mut self) { - call_inner!(self, set_style_attribute); + pub fn set_style_attribute(&mut self, session: &Session) { + call_inner!(self, set_style_attribute, session); } pub fn is_in_error(&self) -> bool { diff --git a/src/handle.rs b/src/handle.rs index 6062d3f13..e18b2a17c 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -378,7 +378,7 @@ impl Handle { pub fn set_stylesheet(&mut self, css: &str) -> Result<(), LoadingError> { let mut stylesheet = Stylesheet::new(Origin::User); stylesheet.parse(css, &UrlResolver::new(None), self.session.clone())?; - self.document.cascade(&[stylesheet]); + self.document.cascade(&[stylesheet], &self.session); Ok(()) } } diff --git a/src/properties.rs b/src/properties.rs index 8d4390d9f..2b077d4cd 100644 --- a/src/properties.rs +++ b/src/properties.rs @@ -840,7 +840,8 @@ impl SpecifiedValues { let mut tok = String::new(); t.to_css(&mut tok).unwrap(); // FIXME: what do we do with a fmt::Error? - rsvg_log!( + rsvg_log_session!( + session, "(ignoring invalid presentation attribute {:?}\n value=\"{}\"\n \ unexpected token '{}')", attr.expanded(), @@ -853,7 +854,8 @@ impl SpecifiedValues { kind: ParseErrorKind::Basic(BasicParseErrorKind::EndOfInput), .. }) => { - rsvg_log!( + rsvg_log_session!( + session, "(ignoring invalid presentation attribute {:?}\n value=\"{}\"\n \ unexpected end of input)", attr.expanded(), @@ -865,7 +867,8 @@ impl SpecifiedValues { kind: ParseErrorKind::Basic(_), .. }) => { - rsvg_log!( + rsvg_log_session!( + session, "(ignoring invalid presentation attribute {:?}\n value=\"{}\"\n \ unexpected error)", attr.expanded(), @@ -877,7 +880,8 @@ impl SpecifiedValues { kind: ParseErrorKind::Custom(ref v), .. }) => { - rsvg_log!( + rsvg_log_session!( + session, "(ignoring invalid presentation attribute {:?}\n value=\"{}\"\n {})", attr.expanded(), value, @@ -954,6 +958,7 @@ impl SpecifiedValues { declarations: &str, origin: Origin, important_styles: &mut HashSet, + session: &Session, ) -> Result<(), ElementError> { let mut input = ParserInput::new(declarations); let mut parser = Parser::new(&mut input); @@ -962,7 +967,7 @@ impl SpecifiedValues { .filter_map(|r| match r { Ok(decl) => Some(decl), Err(e) => { - rsvg_log!("Invalid declaration; ignoring: {:?}", e); + rsvg_log_session!(session, "Invalid declaration; ignoring: {:?}", e); None } }) @@ -1118,7 +1123,7 @@ mod tests { let mut specified = SpecifiedValues::default(); assert!(specified - .parse_style_declarations("", Origin::Author, &mut HashSet::new()) + .parse_style_declarations("", Origin::Author, &mut HashSet::new(), &Session::default()) .is_ok()) } } -- GitLab From cca3638fedf2bb06724fb962c9aee070777d144a Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 23 Aug 2022 17:50:51 -0500 Subject: [PATCH 35/44] Pass a Session to set_attributes() functions Part-of: --- src/element.rs | 4 ++-- src/filter.rs | 3 ++- src/filters/blend.rs | 3 ++- src/filters/color_matrix.rs | 3 ++- src/filters/component_transfer.rs | 5 +++-- src/filters/composite.rs | 3 ++- src/filters/convolve_matrix.rs | 3 ++- src/filters/displacement_map.rs | 3 ++- src/filters/flood.rs | 3 ++- src/filters/gaussian_blur.rs | 3 ++- src/filters/image.rs | 3 ++- src/filters/lighting.rs | 11 ++++++----- src/filters/merge.rs | 5 +++-- src/filters/morphology.rs | 3 ++- src/filters/offset.rs | 3 ++- src/filters/tile.rs | 3 ++- src/filters/turbulence.rs | 3 ++- src/gradient.rs | 14 +++++++------- src/image.rs | 3 ++- src/marker.rs | 3 ++- src/pattern.rs | 2 +- src/shapes.rs | 10 +++++----- src/structure.rs | 13 +++++++------ src/style.rs | 3 ++- src/text.rs | 6 +++--- 25 files changed, 69 insertions(+), 49 deletions(-) diff --git a/src/element.rs b/src/element.rs index 7db33608d..274fc4e94 100644 --- a/src/element.rs +++ b/src/element.rs @@ -71,7 +71,7 @@ pub trait SetAttributes { /// Sets per-element attributes. /// /// Each element is supposed to iterate the `attributes`, and parse any ones it needs. - fn set_attributes(&mut self, _attributes: &Attributes) -> ElementResult { + fn set_attributes(&mut self, _attributes: &Attributes, _session: &Session) -> ElementResult { Ok(()) } } @@ -599,7 +599,7 @@ macro_rules! e { ) -> Element { let mut element_impl = <$element_type>::default(); - let result = element_impl.set_attributes(&attributes); + let result = element_impl.set_attributes(&attributes, session); let element = Element::$element_type(Box::new(ElementInner::new( session, diff --git a/src/filter.rs b/src/filter.rs index 2be886b3e..ac81d66e7 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -15,6 +15,7 @@ use crate::length::*; use crate::node::NodeBorrow; use crate::parsers::{Parse, ParseValue}; use crate::rect::Rect; +use crate::session::Session; use crate::xml::Attributes; /// The node. @@ -70,7 +71,7 @@ impl Filter { } impl SetAttributes for Filter { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "filterUnits") => self.filter_units = attr.parse(value)?, diff --git a/src/filters/blend.rs b/src/filters/blend.rs index c56d68b8d..2928fdfa2 100644 --- a/src/filters/blend.rs +++ b/src/filters/blend.rs @@ -9,6 +9,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::shared_surface::Operator; use crate::xml::Attributes; @@ -59,7 +60,7 @@ pub struct Blend { } impl SetAttributes for FeBlend { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { let (in1, in2) = self.base.parse_two_inputs(attrs)?; self.params.in1 = in1; self.params.in2 = in2; diff --git a/src/filters/color_matrix.rs b/src/filters/color_matrix.rs index a9af7a914..c3c2b5385 100644 --- a/src/filters/color_matrix.rs +++ b/src/filters/color_matrix.rs @@ -10,6 +10,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{NumberList, Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ iterators::Pixels, shared_surface::ExclusiveImageSurface, ImageSurfaceDataExt, Pixel, }; @@ -63,7 +64,7 @@ impl Default for ColorMatrix { #[rustfmt::skip] impl SetAttributes for FeColorMatrix { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; // First, determine the operation type. diff --git a/src/filters/component_transfer.rs b/src/filters/component_transfer.rs index 1716ef21d..31027e6c5 100644 --- a/src/filters/component_transfer.rs +++ b/src/filters/component_transfer.rs @@ -11,6 +11,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::parsers::{NumberList, Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ iterators::Pixels, shared_surface::ExclusiveImageSurface, ImageSurfaceDataExt, Pixel, }; @@ -40,7 +41,7 @@ pub struct ComponentTransfer { } impl SetAttributes for FeComponentTransfer { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; Ok(()) } @@ -214,7 +215,7 @@ macro_rules! func_x { impl SetAttributes for $func_name { #[inline] - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "type") => self.function_type = attr.parse(value)?, diff --git a/src/filters/composite.rs b/src/filters/composite.rs index 2a2e47ed7..1b6904202 100644 --- a/src/filters/composite.rs +++ b/src/filters/composite.rs @@ -9,6 +9,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::shared_surface::Operator as SurfaceOperator; use crate::xml::Attributes; @@ -53,7 +54,7 @@ pub struct Composite { } impl SetAttributes for FeComposite { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { let (in1, in2) = self.base.parse_two_inputs(attrs)?; self.params.in1 = in1; self.params.in2 = in2; diff --git a/src/filters/convolve_matrix.rs b/src/filters/convolve_matrix.rs index bd796730d..54a390c86 100644 --- a/src/filters/convolve_matrix.rs +++ b/src/filters/convolve_matrix.rs @@ -10,6 +10,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{NonNegative, NumberList, NumberOptionalNumber, Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ iterators::{PixelRectangle, Pixels}, shared_surface::ExclusiveImageSurface, @@ -69,7 +70,7 @@ impl Default for ConvolveMatrix { } impl SetAttributes for FeConvolveMatrix { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/displacement_map.rs b/src/filters/displacement_map.rs index b7aaf8654..c88b4eec5 100644 --- a/src/filters/displacement_map.rs +++ b/src/filters/displacement_map.rs @@ -9,6 +9,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{iterators::Pixels, shared_surface::ExclusiveImageSurface}; use crate::xml::Attributes; @@ -49,7 +50,7 @@ pub struct DisplacementMap { } impl SetAttributes for FeDisplacementMap { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { let (in1, in2) = self.base.parse_two_inputs(attrs)?; self.params.in1 = in1; self.params.in2 = in2; diff --git a/src/filters/flood.rs b/src/filters/flood.rs index a6ee723c3..4a596e92d 100644 --- a/src/filters/flood.rs +++ b/src/filters/flood.rs @@ -4,6 +4,7 @@ use crate::element::{ElementResult, SetAttributes}; use crate::node::{CascadedValues, Node}; use crate::paint_server::resolve_color; use crate::rect::IRect; +use crate::session::Session; use crate::xml::Attributes; use super::bounds::BoundsBuilder; @@ -24,7 +25,7 @@ pub struct Flood { } impl SetAttributes for FeFlood { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.base.parse_no_inputs(attrs) } } diff --git a/src/filters/gaussian_blur.rs b/src/filters/gaussian_blur.rs index 114b1d69f..3d4fe9ae0 100644 --- a/src/filters/gaussian_blur.rs +++ b/src/filters/gaussian_blur.rs @@ -11,6 +11,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{NonNegative, NumberOptionalNumber, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ shared_surface::{BlurDirection, Horizontal, SharedImageSurface, Vertical}, EdgeMode, @@ -45,7 +46,7 @@ pub struct GaussianBlur { } impl SetAttributes for FeGaussianBlur { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/image.rs b/src/filters/image.rs index 243bf0cf5..65ddf759c 100644 --- a/src/filters/image.rs +++ b/src/filters/image.rs @@ -9,6 +9,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::ParseValue; use crate::properties::ComputedValues; use crate::rect::Rect; +use crate::session::Session; use crate::surface_utils::shared_surface::SharedImageSurface; use crate::viewbox::ViewBox; use crate::xml::Attributes; @@ -115,7 +116,7 @@ impl Image { } impl SetAttributes for FeImage { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.base.parse_no_inputs(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/lighting.rs b/src/filters/lighting.rs index f1e94beb0..3d4c73a5a 100644 --- a/src/filters/lighting.rs +++ b/src/filters/lighting.rs @@ -21,6 +21,7 @@ use crate::paint_server::resolve_color; use crate::parsers::{NonNegative, NumberOptionalNumber, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ shared_surface::{ExclusiveImageSurface, SharedImageSurface, SurfaceType}, ImageSurfaceDataExt, Pixel, @@ -213,7 +214,7 @@ impl FeDistantLight { } impl SetAttributes for FeDistantLight { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "azimuth") => self.azimuth = attr.parse(value)?, @@ -247,7 +248,7 @@ impl FePointLight { } impl SetAttributes for FePointLight { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "x") => self.x = attr.parse(value)?, @@ -297,7 +298,7 @@ impl FeSpotLight { } impl SetAttributes for FeSpotLight { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "x") => self.x = attr.parse(value)?, @@ -332,7 +333,7 @@ fn transform_dist(t: Transform, d: f64) -> f64 { } impl SetAttributes for FeDiffuseLighting { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { @@ -377,7 +378,7 @@ impl DiffuseLighting { } impl SetAttributes for FeSpecularLighting { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/merge.rs b/src/filters/merge.rs index 747465982..e2e9be658 100644 --- a/src/filters/merge.rs +++ b/src/filters/merge.rs @@ -7,6 +7,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::parsers::ParseValue; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::shared_surface::{Operator, SharedImageSurface, SurfaceType}; use crate::xml::Attributes; @@ -51,14 +52,14 @@ impl Default for FeMerge { } impl SetAttributes for FeMerge { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.base.parse_no_inputs(attrs) } } impl SetAttributes for FeMergeNode { #[inline] - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { if let expanded_name!("", "in") = attr.expanded() { self.in1 = attr.parse(value)?; diff --git a/src/filters/morphology.rs b/src/filters/morphology.rs index 7b4dbf377..8a6c0725a 100644 --- a/src/filters/morphology.rs +++ b/src/filters/morphology.rs @@ -11,6 +11,7 @@ use crate::node::Node; use crate::parsers::{NonNegative, NumberOptionalNumber, Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ iterators::{PixelRectangle, Pixels}, shared_surface::ExclusiveImageSurface, @@ -50,7 +51,7 @@ pub struct Morphology { } impl SetAttributes for FeMorphology { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/offset.rs b/src/filters/offset.rs index 1f97b6126..d60542a54 100644 --- a/src/filters/offset.rs +++ b/src/filters/offset.rs @@ -7,6 +7,7 @@ use crate::node::Node; use crate::parsers::ParseValue; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::xml::Attributes; use super::bounds::BoundsBuilder; @@ -32,7 +33,7 @@ pub struct Offset { } impl SetAttributes for FeOffset { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/filters/tile.rs b/src/filters/tile.rs index 8c397b3ed..45549a71e 100644 --- a/src/filters/tile.rs +++ b/src/filters/tile.rs @@ -4,6 +4,7 @@ use crate::element::{ElementResult, SetAttributes}; use crate::node::Node; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::xml::Attributes; use super::bounds::BoundsBuilder; @@ -27,7 +28,7 @@ pub struct Tile { } impl SetAttributes for FeTile { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.params.in1 = self.base.parse_one_input(attrs)?; Ok(()) } diff --git a/src/filters/turbulence.rs b/src/filters/turbulence.rs index 2f40f7f26..cb962c45c 100644 --- a/src/filters/turbulence.rs +++ b/src/filters/turbulence.rs @@ -9,6 +9,7 @@ use crate::node::{CascadedValues, Node}; use crate::parsers::{NonNegative, NumberOptionalNumber, Parse, ParseValue}; use crate::properties::ColorInterpolationFilters; use crate::rect::IRect; +use crate::session::Session; use crate::surface_utils::{ shared_surface::{ExclusiveImageSurface, SurfaceType}, ImageSurfaceDataExt, Pixel, PixelOps, @@ -74,7 +75,7 @@ impl Default for Turbulence { } impl SetAttributes for FeTurbulence { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { self.base.parse_no_inputs(attrs)?; for (attr, value) in attrs.iter() { diff --git a/src/gradient.rs b/src/gradient.rs index b1e6813e7..a45e8a850 100644 --- a/src/gradient.rs +++ b/src/gradient.rs @@ -17,6 +17,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::paint_server::resolve_color; use crate::parsers::{Parse, ParseValue}; use crate::properties::ComputedValues; +use crate::session::Session; use crate::transform::{Transform, TransformAttribute}; use crate::unit_interval::UnitInterval; use crate::xml::Attributes; @@ -65,7 +66,7 @@ pub struct Stop { } impl SetAttributes for Stop { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { if let expanded_name!("", "offset") = attr.expanded() { self.offset = attr.parse(value)?; @@ -515,7 +516,7 @@ impl RadialGradient { } impl SetAttributes for Common { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "gradientUnits") => self.units = attr.parse(value)?, @@ -540,8 +541,8 @@ impl SetAttributes for Common { } impl SetAttributes for LinearGradient { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { - self.common.set_attributes(attrs)?; + fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult { + self.common.set_attributes(attrs, session)?; for (attr, value) in attrs.iter() { match attr.expanded() { @@ -632,8 +633,8 @@ impl_gradient!(LinearGradient, RadialGradient); impl_gradient!(RadialGradient, LinearGradient); impl SetAttributes for RadialGradient { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { - self.common.set_attributes(attrs)?; + fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult { + self.common.set_attributes(attrs, session)?; // Create a local expanded name for "fr" because markup5ever doesn't have built-in let expanded_name_fr = ExpandedName { ns: &Namespace::from(""), @@ -716,7 +717,6 @@ impl ResolvedGradient { mod tests { use super::*; use crate::node::{Node, NodeData}; - use crate::session::Session; use markup5ever::{namespace_url, ns, QualName}; #[test] diff --git a/src/image.rs b/src/image.rs index b674a51e9..64e948b3a 100644 --- a/src/image.rs +++ b/src/image.rs @@ -14,6 +14,7 @@ use crate::length::*; use crate::node::{CascadedValues, Node, NodeBorrow}; use crate::parsers::ParseValue; use crate::rect::Rect; +use crate::session::Session; use crate::xml::Attributes; /// The `` element. @@ -27,7 +28,7 @@ pub struct Image { } impl SetAttributes for Image { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "preserveAspectRatio") => self.aspect = attr.parse(value)?, diff --git a/src/marker.rs b/src/marker.rs index 87c0135f4..e5c584f05 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -20,6 +20,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw}; use crate::parsers::{Parse, ParseValue}; use crate::path_builder::{arc_segment, ArcParameterization, CubicBezierCurve, Path, PathCommand}; use crate::rect::Rect; +use crate::session::Session; use crate::transform::Transform; use crate::viewbox::*; use crate::xml::Attributes; @@ -199,7 +200,7 @@ impl Marker { } impl SetAttributes for Marker { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "markerUnits") => self.units = attr.parse(value)?, diff --git a/src/pattern.rs b/src/pattern.rs index 7152c13e6..28fba156f 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -125,7 +125,7 @@ pub struct Pattern { } impl SetAttributes for Pattern { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "patternUnits") => self.common.units = attr.parse(value)?, diff --git a/src/shapes.rs b/src/shapes.rs index d7a161b4c..8af6a362e 100644 --- a/src/shapes.rs +++ b/src/shapes.rs @@ -248,7 +248,7 @@ pub struct Path { impl_draw!(Path); impl SetAttributes for Path { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { if attr.expanded() == expanded_name!("", "d") { let mut builder = PathBuilder::default(); @@ -256,7 +256,7 @@ impl SetAttributes for Path { // FIXME: we don't propagate errors upstream, but creating a partial // path is OK per the spec - rsvg_log!("could not parse path: {}", e); + rsvg_log_session!(session, "could not parse path: {}", e); } self.path = Rc::new(builder.into_path()); } @@ -339,7 +339,7 @@ pub struct Polygon { impl_draw!(Polygon); impl SetAttributes for Polygon { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { if attr.expanded() == expanded_name!("", "points") { self.points = attr.parse(value)?; @@ -364,7 +364,7 @@ pub struct Polyline { impl_draw!(Polyline); impl SetAttributes for Polyline { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { if attr.expanded() == expanded_name!("", "points") { self.points = attr.parse(value)?; @@ -392,7 +392,7 @@ pub struct Line { impl_draw!(Line); impl SetAttributes for Line { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "x1") => self.x1 = attr.parse(value)?, diff --git a/src/structure.rs b/src/structure.rs index da0457284..cfc8cbcd0 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -16,6 +16,7 @@ use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw}; use crate::parsers::{Parse, ParseValue}; use crate::properties::ComputedValues; use crate::rect::Rect; +use crate::session::Session; use crate::viewbox::*; use crate::xml::Attributes; @@ -264,7 +265,7 @@ impl Svg { } impl SetAttributes for Svg { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "preserveAspectRatio") => { @@ -345,7 +346,7 @@ impl Default for Use { } impl SetAttributes for Use { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { ref a if is_href(a) => set_href( @@ -431,7 +432,7 @@ impl Symbol { } impl SetAttributes for Symbol { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "preserveAspectRatio") => { @@ -462,7 +463,7 @@ impl ClipPath { } impl SetAttributes for ClipPath { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { let result = attrs .iter() .find(|(attr, _)| attr.expanded() == expanded_name!("", "clipPathUnits")) @@ -525,7 +526,7 @@ impl Mask { } impl SetAttributes for Mask { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { expanded_name!("", "x") => self.x = attr.parse(value)?, @@ -550,7 +551,7 @@ pub struct Link { } impl SetAttributes for Link { - fn set_attributes(&mut self, attrs: &Attributes) -> ElementResult { + fn set_attributes(&mut self, attrs: &Attributes, _session: &Session) -> ElementResult { for (attr, value) in attrs.iter() { match attr.expanded() { ref a if is_href(a) => set_href(a, &mut self.link, value.to_owned()), diff --git a/src/style.rs b/src/style.rs index 0d566c5be..8c3028e92 100644 --- a/src/style.rs +++ b/src/style.rs @@ -4,6 +4,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns}; use crate::element::{Draw, ElementResult, SetAttributes}; use crate::error::*; +use crate::session::Session; use crate::xml::Attributes; /// Represents the syntax used in the