deeplook / svglib

Read SVG files and convert them to other formats.
GNU Lesser General Public License v3.0
325 stars 79 forks source link

HEX/RGB colors are getting corrupted #402

Open jdefockert opened 1 month ago

jdefockert commented 1 month ago

When using SVGlib to convert plotly SVG charts created using Kaleido.

Hex colors containing the value "01" are converted to "FF" causing the color to change from what it is originally in the SVG.

Example: F57D01 (Orange) changes into F57DFF (Pink)

010063 (Dark Blue) changes into FF0063 (Pinkish red)

Using: Python 3.11.9, SVGlib 1.5.1, Reportlab 4.2.5, Plotly 5.24.1, Kaleido 0.1.0.post1

Thank you in advance for looking into it.

github-actions[bot] commented 1 month ago

Thank you for raising your first issue! Your help to improve svglib is much appreciated!

claudep commented 1 month ago

Thanks for the report. Having a smallest possible sample to reproduce would be very appreciated!

jdefockert commented 1 month ago

Please find a sample that reproduces the error:

import plotly.graph_objects as go
import io
from svglib.svglib import svg2rlg
from reportlab.lib.pagesizes import letter
from reportlab.lib.units import inch
from reportlab.platypus import BaseDocTemplate, PageTemplate, Frame, Spacer
from plotly.io import to_image, write_image

# Create a function to generate a chart using Plotly and export to SVG format
def generate_plotly_chart_as_svg(title, color):
    fig = go.Figure()
    fig.add_trace(go.Scatter(x=[1, 2, 3, 4], y=[10, 11, 12, 13], mode='lines+markers', marker_color=color, name=title))
    fig.update_layout(title=title)

    # Export as an in-memory SVG
    buffer = io.BytesIO()
    svg_data = to_image(fig, format="svg")
    write_image(fig, title.replace(' ','_') + '.svg')
    buffer.write(svg_data)
    buffer.seek(0)
    return buffer

# Function to convert SVG into ReportLab Drawable and scale it to the specified dimensions
def svg_to_flowable(svg_buffer, target_width, target_height):
    drawing = svg2rlg(svg_buffer)

    # Scale the drawing to fit the desired width and height
    scale_x = target_width / drawing.width
    scale_y = target_height / drawing.height
    scale = min(scale_x, scale_y)  # Use the smaller scale to preserve aspect ratio

    # Apply scaling
    drawing.width = drawing.width * scale
    drawing.height = drawing.height * scale
    drawing.scale(scale, scale)

    return drawing

# Create 3 Plotly charts and export as SVGs
chart1_svg = generate_plotly_chart_as_svg("Dark Blue Chart", "#010063")
chart2_svg = generate_plotly_chart_as_svg("Orange Chart", "#F57D01")

# Create the document with letter-size pages
doc = BaseDocTemplate("color_corruption_example.pdf", pagesize=letter)
doc.title('Color corruption example')
frame = Frame(0.5 * inch, 0.5 * inch, 7.5 * inch, 10 * inch)  # Margins set at 0.5 inch
doc.addPageTemplates([PageTemplate(id="chart_template", frames=[frame])])

# Create a list to hold all content elements
elements = []

elements.append(svg_to_flowable(chart1_svg, 7.5 * inch, 3 * inch))  # Full-width chart

elements.append(Spacer(1, 0.5 * inch))

elements.append(svg_to_flowable(chart2_svg, 7.5 * inch, 3 * inch))  # Full-width chart

# Build the PDF document
doc.build(elements)

print("PDF created successfully!")
claudep commented 1 month ago

Thanks for the procedure, I also thought about a SVG sample, if possible.

jdefockert commented 1 month ago

I updated the example so that it saves the generated charts as SVG as well.

jdefockert commented 1 month ago

Uploaded them here for your convenience as well. Orange_Chart Dark_Blue_Chart

claudep commented 1 month ago

Many thanks for the sample files.

I was able to identify this issue as a bug in reportlab color converter (when any color in the rgb() expression has the value 1). This patch in reportlab should fix the issue:

diff --git a/src/reportlab/lib/colors.py b/src/reportlab/lib/colors.py
index 2c409bc8..f2c9ba80 100644
--- a/src/reportlab/lib/colors.py
+++ b/src/reportlab/lib/colors.py
@@ -16,6 +16,8 @@ rhese can be constructed from several popular formats.  We also include
 These tests are here because doctest cannot find them otherwise.
 >>> toColor('rgb(128,0,0)')==toColor('rgb(50%,0%,0%)')
 True
+>>> toColor('rgb(255,1,0)')==Color(1,.003922,0)
+True
 >>> toColor('rgb(50%,0%,0%)')!=Color(0.5,0,0,1)
 True
 >>> toColor('hsl(0,100%,50%)')==toColor('rgb(255,0,0)')
@@ -787,7 +789,7 @@ class cssParse:
         v = v.strip()
         try:
             c=float(v)
-            if 0<c<=1: c *= 255
+            if 0<c<1: c *= 255
             return int(min(255,max(0,c)))/255.
         except:
             raise ValueError('bad argument value %r in css color %r' % (v,self.s))

I don't know if the support of a rgb() syntax with values between 0 and 1 was something deliberate or not. I don't find any reference of such a syntax in the CSS specs.

@replabrobin, would you be able to upstream that fix in reportlab, or would you like a post on the mailing list?

replabrobin commented 1 month ago

I think the intention was to support the case where people used only 0-1 values for all rgba values. The assumption is then that these represent fractions of the whole range. Clearly not well thought out; blame me.

Is there a soltution that's better. Probably we should check that all values are in the range [0, 1] before assuming the 255 factor.

claudep commented 1 month ago

I guess that rgb(1, 1, 1) might be ambiguous in any case. Do you think it's worth supporting the rgb(0.3, 0.2, 0.7) use case at all, as it doesn't seem to conform to the specs?