Skip to content
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

Enhancement to improve the svg-image-handling and the usage of data-url #1386 #1387

Merged

Conversation

speckyspooky
Copy link
Contributor

enhancement to improve the svg-image-handling and the usage of data-url #1386

@speckyspooky speckyspooky self-assigned this Aug 8, 2023
@speckyspooky speckyspooky added the Enhancement Small change to improve the current supported functionality label Aug 8, 2023
@speckyspooky speckyspooky added this to the 4.14 milestone Aug 8, 2023
@speckyspooky
Copy link
Contributor Author

For documentation reasons of the test results I will add some screens according to the issue ticket
and also the used BIRT-report with the 2 used images (png & svg). All other used images included on the report like data-URL or added via https-link (URL).

Screen of the designer with the change

enh-designer-preview-compare-data-url-and-svg

Screen of the output formats HTML, PDF and DOCX with the change

enh-image-compare-data-url-and-svg

Test report & images

Hint: before the report can be used the local path to the images must be changed.
enhanced_handling_image_overview.zip

Output: PDF & DOCX with the change

enhanced_handling_image_overview.zip

@hvbtup
Copy link
Contributor

hvbtup commented Aug 9, 2023

Looking at the output, it looks very well.
I'm very busy ATM and will be on holiday in weeks 33 and 34, so I don't have the time to look at the code.
I recommend to let someone else review the code.

@speckyspooky
Copy link
Contributor Author

speckyspooky commented Aug 9, 2023

@hvbtup

Thanks for your first look.
One additional information, I double checked the PDF-file to verify the SVG-graphics (to have real SVG images in there).
The result is that the SVGs are added like original SVGs and not transformed to raster images. The raster images are only uses on the office types (PPTX, DOCX, XSLX).

Below is the compare with Adobe reader zoom=600% to see the raster details in compare to the SVG.
I checked it again with the Inkscape graphic tool and imported the PDF-document. I was able to extract the svg-path-elements of the SVG-images and the png-images was a standard raster image. So I think we got it to have real SVG-images on the PDF-document.

svg-raster

Copy link
Contributor

@wimjongman wimjongman left a comment

Choose a reason for hiding this comment

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

This code looks good to me. Can you add tests?

@speckyspooky
Copy link
Contributor Author

Thanks for your check.

Yes, I can do it and set it on my list.
(The next step will be to remove some standard warnings of the changed classes on an own PR.)

@ilnur-z
Copy link

ilnur-z commented Dec 18, 2023

i have NPE on method org.eclipse.birt.report.engine.util.SvgFile.isSvg(String) then uri is null.
I think that because some brackets needed:
if (uri != null && ( uri.endsWith(".svg") || uri.toLowerCase().contains(URL_IMAGE_TYPE_SVG) ) )
(just like on method org.eclipse.birt.report.engine.util.SvgFile.isSvg(String, String, String))

I've created issue #1525

@speckyspooky speckyspooky deleted the enhance_image_handling branch March 28, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Small change to improve the current supported functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants