New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opacity attribute is ignored when using fill=url() #272

Closed
Anubis88 opened this Issue Feb 18, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@Anubis88
Copy link
Contributor

Anubis88 commented Feb 18, 2015

When the fill attribute is using a 'url' then the opacity attribute is ignored. Here is an example svg image to prove this. Opening with Safari (WebKit) displays fine but rendering with SVGKit gets a wrong result.

Example SVG:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
     width="212.6px" height="212.6px" viewBox="0 0 212.6 212.6" enable-background="new 0 0 212.6 212.6" xml:space="preserve">
<radialGradient id="SVGID_1_" cx="106.3" cy="106.3" r="106.3" gradientUnits="userSpaceOnUse">
    <stop  offset="0" style="stop-color:#FFFFFF"/>
    <stop  offset="1" style="stop-color:#000000"/>
</radialGradient>
<circle opacity="0.2" fill="url(#SVGID_1_)" cx="106.3" cy="106.3" r="106.3"/>
</svg>

WebKit (Correct rendering):
webkit

SVGKit (Wrong rendering):
svgkit

@Anubis88

This comment has been minimized.

Copy link
Contributor

Anubis88 commented Feb 24, 2015

Just wondering if this issue could this issue be fixed. I'd really need it for my project.
Thanks in advance.

@adamgit

This comment has been minimized.

Copy link
Contributor

adamgit commented Feb 24, 2015

If you need it, you're welcome to fix it, or offer to pay someone else to fix it if you don't have time.

Otherwise, it will wait until someone else decides it's more fun to fix a bug for you, than to work on their own project.

@Anubis88

This comment has been minimized.

Copy link
Contributor

Anubis88 commented Feb 26, 2015

I'll just assume that you had a bad day, that's all. :)

I've fixed the issue. If you tell me how to commit the fix I'd like to contribute to the project. Thank you.

@MaddTheSane

This comment has been minimized.

Copy link
Contributor

MaddTheSane commented Feb 26, 2015

Fixes are usually done via pull requests: just fork the repository, pull your fork, commit your changes, push your changes to your fork, compare, create pull request.

@adamgit

This comment has been minimized.

Copy link
Contributor

adamgit commented Feb 26, 2015

EDIT: what MTS said :)

Great. All you need do is:

  1. Fork the project
  2. Run the demo project and check your chnage hasn't broken any of the previously-working images
  3. Commit to your fork
  4. On the github page for your fork, there will be a big butotn at the top "submit pull request"

...and it's all automatic from there. Modulo one of the rest of us checking your source chagnes

@Anubis88

This comment has been minimized.

Copy link
Contributor

Anubis88 commented Feb 26, 2015

Ok I just did, I hope it worked.

@adamgit

This comment has been minimized.

Copy link
Contributor

adamgit commented Feb 26, 2015

Worked fine.

Would be great to get another pull request which adds your example file to the Demos project. That requires:

  1. add it to the folder with the other SVGs
  2. edit the Sample Licenses.txt to explicitly say its yours, and you're releasing it as public domain
  3. edit the Demo project where it has a list of SVG filenames (I usually do a project-wide search for "monkey", it quickly finds the NSString array with all names), and add your new filename to the array.
@adamgit

This comment has been minimized.

Copy link
Contributor

adamgit commented Feb 26, 2015

PS: Orta today recommended we try using FBSnapshotTestCase to do automated tests of all the sample SVGs. Looks like all we need to do is modify the Demos project to automatically zoom out/in each image when rendering (so it fits the available area), and add a dozen or so lines of code to trigger the automatic tests. Would be especially good for something like this where the "wrong" result looks almost "right"

@Anubis88

This comment has been minimized.

Copy link
Contributor

Anubis88 commented Feb 27, 2015

Added demo file as requested.

@Anubis88 Anubis88 closed this Mar 3, 2015

dgileadi added a commit to dgileadi/SVGKit that referenced this issue Mar 16, 2015

Merge branch 'original-2.x' into no-clip-children
* original-2.x:
  Fixed a bug with 'Smooth Curveto' and 'Smooth Quadratic Curveto' paths where the relative point was never updated for the next commands producing a distorted result. When the command letter is missing on subsequent commands (if the same command is used multiple times in a row) (SVGKit#273)
  Added demo example file to test radial gradient opacity fix (SVGKit#272).
  Fixed bug: opacity attribute is ignored when using fill=url() (SVGKit#272)
  Fixed crash on iOS 8 if any CAShapeLayer is empty
  Fixed crash when changing a SVGKLayeredImageView image
  Fixed bug on conversion from millimeter to inches and inches to milimeter
  Improve parsing to avoid redundant copying and stack allocation, which can avoid errors with files that have really large embedded images.

dgileadi added a commit to dgileadi/SVGKit that referenced this issue Mar 18, 2015

Merge branch 'original-2.x' into 2.x
* original-2.x:
  Add a version of cascadedValueForStylableProperty that allows you to choose whether the property is inherited; change code to use that version for properties that don't inherit.
  Improve the sample with animation (what I thought was "proof" was actually a rendering bug in the code); fix that rendering bug so that clip-path on <g> elements gets its frame adjusted after the SVGGElement calculates its own frame.
  Add an SVG file to the demo app showing that the fix is correct and that the current behavior is incorrect.
  Fixed a bug with 'Smooth Curveto' and 'Smooth Quadratic Curveto' paths where the relative point was never updated for the next commands producing a distorted result. When the command letter is missing on subsequent commands (if the same command is used multiple times in a row) (SVGKit#273)
  Added demo example file to test radial gradient opacity fix (SVGKit#272).
  Fixed bug: opacity attribute is ignored when using fill=url() (SVGKit#272)
  Fixed crash on iOS 8 if any CAShapeLayer is empty
  Fixed crash when changing a SVGKLayeredImageView image
  Fixed bug on conversion from millimeter to inches and inches to milimeter
  Only apply clipping masks to elements that have the clip-path attribute, not (redundantly) to their children too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment