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

Preliminary PDF/UA support #1972

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

hvbtup
Copy link
Contributor

@hvbtup hvbtup commented Nov 14, 2024

As I said yesterday, I think my changes are now in a state that they do no harm to the master branch; creating normal PDFs should work exactly as before.
It is now possible to at least generate valid single-page PDF/UA files.
The smiling sun image is my IP, I created it a few years ago for a game experiment.

I cannot split this into more smaller commits in a reasonable way.

Note: The Java code certainly has some room for improvements.
In several cases I used the instanceof operator, because it was the easiest solution and avoids having to change interfaces.

BTW most of the changes in EngineIRVisitor.java were suggested by the IDE to remove warnings.

@hvbtup hvbtup added Help wanted Enhancement Small change to improve the current supported functionality labels Nov 14, 2024
@hvbtup
Copy link
Contributor Author

hvbtup commented Nov 14, 2024

@wimjongman

One of the tests is failing:
EngineIRIOTest.testIO:39->doTestIO:72 EngineIRIOTest, doTestIO(compare: golden, value) - designName: action_test.rptdesign expected:<... <label id="8" t[ag-type="P" text="uri-expr"> ... (output shortened) ... but got:<... <label id="8" t[ext="uri-expr"> ...

This obviously means that a tag-type attribute got lost. This might be related to one of my changes: AFAIK the tags are case-sensitive, for example it must be P and not p.
I changed this in model/org.eclipse.birt.report.model/src/org/eclipse/birt/report/model/elements/rom.def

Probably now the value of P is not written to the file because P equals the default value.

So I think I have to modify the "golden" file, do I?

@wimjongman
Copy link
Contributor

@hvbtup Yes, go ahead.

@hvbtup
Copy link
Contributor Author

hvbtup commented Nov 14, 2024

Nah. By examining the test, I see that the "golden" image is not a file. It is the report, loaded with ReportParser().parse.
This is then serialized with EngineIRWriter and deserialized with EngineIReader. The first and the final report are both saved with ReportDesignWriter. And obviously ReportDesignWriter actually writes the tags into the rptdesign file, but when using the long way through the IR, they get lost.

@speckyspooky
Copy link
Contributor

@hvbtup
I have dine a first review to understand your changes and I think I understand your steps. But I will do a second read of it. The only thing in generally I saw some comments in German and file name "Sonne.png". It would be good if we have all comments in English and the image name is optional for me.

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 Help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants