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

Improve readability of tables #1510

Merged
merged 5 commits into from
May 22, 2024
Merged

Conversation

pavelbraginskiy
Copy link
Member

Adds shading to every other row of many of the harder to read tables to make them easier to scan horizontally.
In order to not affect official record sheets, shading must be enabled from an option in the Configuration menu.

An example of the shaded tables:
image
See here for an example of every covered unit type: testsheets.pdf

Problems

Enabling the option slows export/print of record sheets significantly, from 1-2 seconds to around half a minute per sheet. Since this doesn't affect users not using the option, which is the default, I don't think this is a blocker. I have no idea why this happens, and could use some insight.

The additional tables from Print reference tables and the fluff-image-replacement tables aren't shaded, and it would be nice if they were. These tables aren't in SVGs and would need a different, programmatic approach in order to be shaded.

@Sleet01
Copy link
Collaborator

Sleet01 commented May 22, 2024

@pavelbraginskiy do you get the same slowdown if you select "Print" but then print to file? This should produce a PostScript file (.ps) rather than a PDF, and may use a different code path for conversion.

@pavelbraginskiy
Copy link
Member Author

@pavelbraginskiy do you get the same slowdown if you select "Print" but then print to file? This should produce a PostScript file (.ps) rather than a PDF, and may use a different code path for conversion.

I hadn't thought to try it, I assumed it would be the same way. Exporting is actually super fast, printing takes forever.

Still no idea why though...

@Sleet01
Copy link
Collaborator

Sleet01 commented May 22, 2024

I copied one of your updated tables and (templates_us/tables_tank.svg) into 0.49.19.1 and didn't see any slowdown while exporting the file. You may have an issue in your new code rather than the .svg conversion itself.

@Sleet01
Copy link
Collaborator

Sleet01 commented May 22, 2024

@pavelbraginskiy do you get the same slowdown if you select "Print" but then print to file? This should produce a PostScript file (.ps) rather than a PDF, and may use a different code path for conversion.

I hadn't thought to try it, I assumed it would be the same way. Exporting is actually super fast, printing takes forever.

Still no idea why though...

This may be related to how PDFs represent filled rectangles, and the printer drivers. I found several instances of people asking about this specific SVG use case and slowdowns.

@pavelbraginskiy
Copy link
Member Author

pavelbraginskiy commented May 22, 2024

I wonder if it makes sense to rasterize the rectangles, then. Seems silly, but if it helps performance...
I might experiment with it later.

@Sleet01
Copy link
Collaborator

Sleet01 commented May 22, 2024

I wonder if it makes sense to rasterize the rectangles, then. Seems silly, but if it helps performance... I might experiment with it later.

It'd be good to get a sampling of other folks' print performance before doing much more work. I'll do a proper review after lunch, then see if we can just pull this. If it's broken for most testers we can consider more changes or fixes at that point.

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Recommend pulling and testing on as many different machines as possible to narrow down scope of printing/exporting slow-down.
Worst case, reverting just the .svg files will fix the issue.

@pavelbraginskiy
Copy link
Member Author

Worst case, reverting just the .svg files will fix the issue.

To be clear, if the option is not enabled, and I believe it's off by default, there is no performance impact.

@Sleet01 Sleet01 merged commit 5b1e7c7 into MegaMek:master May 22, 2024
4 checks passed
@pavelbraginskiy
Copy link
Member Author

So I tried rasterizing the shading. It does improve performance back to near-instant, but it make the rendering of the text under the shading look awful. It isn't a solution.

@Sleet01
Copy link
Collaborator

Sleet01 commented May 23, 2024

So I tried rasterizing the shading. It does improve performance back to near-instant, but it make the rendering of the text under the shading look awful. It isn't a solution.

I blame HP; I don't have any specific reason, I am just willing to blame them :)

Let's see how the initial update works for folks, and then assess from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants