Redesign of SvgPaintServer decision in SvgPaintServerFactory. #564
Conversation
| _fallbackServer = FallbackServer; | ||
| if (!(_fallbackServer is SvgColourServer || | ||
| (_fallbackServer is SvgDeferredPaintServer && string.Equals(((SvgDeferredPaintServer)_fallbackServer).DeferredId, "currentColor")))) | ||
| _fallbackServer = Inherit; |
There was a problem hiding this comment.
Same behavior as major browsers.
|
|
||
| namespace Svg | ||
| { | ||
| internal class SvgDeferredPaintServerFactory : SvgPaintServerFactory |
There was a problem hiding this comment.
I create this Factory for xlink:href because SvgPaintServerFactory cannot make an accurate decision.
There was a problem hiding this comment.
Can you please add a bit of explanation to the factory or the ConvertFrom method? It took some time for me to understand why it is needed...
| <?xml version="1.0" encoding="utf-8"?> | ||
| <svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="default" width="80" height="80"> | ||
| <defs> | ||
| <linearGradient id="mycolor" xlink:href="#ff0000" /> |
There was a problem hiding this comment.
ID like color format.
tebjan
force-pushed
the
master
branch
2 times, most recently
from
55d5c65
to
7de872a
Compare
| @@ -9,6 +9,7 @@ namespace Svg | |||
| /// <summary> | |||
| /// A wrapper for a paint server has a fallback if the primary server doesn't work. | |||
| /// </summary> | |||
| [Obsolete("Will be removed.")] | |||
There was a problem hiding this comment.
It would be nice to have an explanation or something like "Use xxx instead" as a message instead of "Will be removed" (which is already the meaning of Obsolete).
| } | ||
| else if (value == "inherit") | ||
| { | ||
| else if (string.Equals(colorValue, "none", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
I would prefer to use the instance methods here, e.g. colorValue.Equals("none", StringComparison.OrdinalIgnoreCase), though that is a matter of taste.
| else if (value == "inherit") | ||
| { | ||
| else if (string.Equals(colorValue, "none", StringComparison.OrdinalIgnoreCase)) | ||
| // none |
There was a problem hiding this comment.
self-explanatory - no comments needed here
There was a problem hiding this comment.
This looks good to me, apart from some minor comment issues, though it would be nice if someone else could have a look (@gvheertum, @wieslawsoltes ?)
- It doesn't work properly with certain IRIs, so fix it in another way. - ref. svg-net#455.
| i++; | ||
| } | ||
| return count; | ||
| #if NETSTANDARD20 |
There was a problem hiding this comment.
I am trying to rebase.
But I do not understand policy of netstandard2.0 support.(#509 )
If disable SvgColourConverter, SvgPaintServerFactory will hardly work.
So is this all right now ?
There was a problem hiding this comment.
Netstandard2 is not fully supported yet (especially SvgColourConverter), so this looks ok to me.
@wieslawsoltes - can you also have a look?
- Make up for PR svg-net#564. - Fixes part of svg-net#779.
- Pass pservers-pattern-03-f.svg.
- Corrects PR #564 - W3C test file pservers-pattern-03-f.svg now correctly rendered
Reference Issue
Ref. #114 and #455 .
What does this implement/fix? Explain your changes.
SvgPaintServer.InheritandNotSetfromSvgColourServertoSvgPaintServer.Any other comments?