Skip to content

Add SR2025 Venue Maps to the Competition event page #610

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

Closed
wants to merge 1 commit into from

Conversation

PeterJCLaw
Copy link
Member

These are extracted from the CI build of the floorplans repo:

Files 'map-with-teams.{pdf,png}' renamed to 'venue-map.{pdf,png}'.

Contributes to srobo/tasks#1426.

These are extracted from the CI build of the floorplans repo:
- srobo/comp-floorplans@e0c7ce2
- https://github.com/srobo/comp-floorplans/actions/runs/13957873494/artifacts/2784492185

Files 'map-with-teams.{pdf,png}' renamed to 'venue-map.{pdf,png}'.
@PeterJCLaw PeterJCLaw force-pushed the sr2025-publish-venue-map branch from 12ba070 to c733718 Compare March 25, 2025 19:47
@PeterJCLaw PeterJCLaw changed the title Add SR2024 Venue Maps to the Competition event page Add SR2025 Venue Maps to the Competition event page Mar 25, 2025
@WillB97
Copy link
Contributor

WillB97 commented Mar 25, 2025

It would be better to take artifacts from the release.

@PeterJCLaw
Copy link
Member Author

It would be better to take artifacts from the release.

Huh, hadn't spotted that there were releases on that repo. The commit in question however is the commit which is tagged as the latest release, so is it not safe to assume that the artefacts are the same? (I can't actually see how the artefacts get from the CI build to the release, though I'd strongly hope that they're not being modified in that process).

@PeterJCLaw
Copy link
Member Author

It would be better to take artifacts from the release.

Huh, hadn't spotted that there were releases on that repo. The commit in question however is the commit which is tagged as the latest release, so is it not safe to assume that the artefacts are the same? (I can't actually see how the artefacts get from the CI build to the release, though I'd strongly hope that they're not being modified in that process).

Comparing the assets: it seems that the PNG is the same, however the PDF is different. Do we know what's changed in the PDF on the release? (I can't immediately see any differences, though would very much prefer to use an asset which has clear provenance)

PeterJCLaw added a commit to srobo/competition-website that referenced this pull request Mar 26, 2025
@PeterJCLaw
Copy link
Member Author

PeterJCLaw commented Mar 26, 2025

For clarity: I'm not planning to update this PR to use the assets from the release -- the files there appear to be in error vs CI and we should prefer CI generated assets (as that ensures they match the source).

Copy link
Contributor

@WillB97 WillB97 left a comment

Choose a reason for hiding this comment

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

Please use the artifacts from the release, I have just fixed the text rendering in the PDF.

Note: the artifacts of the release were generated from the action on the commit pointed to by the release, not the tag that was created by the release.

@PeterJCLaw
Copy link
Member Author

PeterJCLaw commented Mar 26, 2025

Please use the artifacts from the release, I have just fixed the text rendering in the PDF.

Note: the artifacts of the release were generated from the action on the commit pointed to by the release, not the tag that was created by the release.

I'm sorry, I don't fully understand what's meant by either of these statements.

Please use the artifacts from the release, I have just fixed the text rendering in the PDF.

Do you mean that you have pushed commits which fix the rendering? (I don't see any new commits) Or do you mean that you have manually changed the PDF asset on the release to fix something?
If the latter then that's the sort of thing I don't think we should be doing as it removes the provenance chain and makes the assets hard to reproduce. We should strongly favour fixing CI so that CI produces usable assets reliably.

Note: the artifacts of the release were generated from the action on the commit pointed to by the release, not the tag that was created by the release.

Could you clarify by linking to the Actions run which you're referring to here? The run I pulled the assets from is https://github.com/srobo/comp-floorplans/actions/runs/13957873494/, which appears to be the one attached to the tag. The other actions run ([edited copy/pasta failed] https://github.com/srobo/comp-floorplans/actions/runs/13955524761) produced the same PNG but different PDF. The PDF on the release however matches neither of those.

@WillB97
Copy link
Contributor

WillB97 commented Mar 26, 2025

This action was used: https://github.com/srobo/comp-floorplans/actions/runs/13955524761.

I have manually fixed font rendering on the PDF using the SVG from this output.

@PeterJCLaw
Copy link
Member Author

What needed fixing? Both actions runs produce identical SVGs. All three PDFs also look fine to me.

If there's something wrong with how the CI produces the assets please could we at least record that as an issue on the repo along with the steps being used to address it? We shouldn't be relying on opaque manual steps as they make it harder for other people to understand processes and are likely to be forgotten in future. If we need to rely on them we should at least document the need and what they are.

@PeterJCLaw
Copy link
Member Author

PeterJCLaw commented Mar 26, 2025

Meta: would it be worth moving to a quick chat for this? I'm conscious that back & forths on a PR easily end up feeling antagonistic through trivial misunderstandings. I accept there are probably good reasons for your request/approach here, I'm just trying to understand what it is and how we ensure things are clear for the future.

@PeterJCLaw
Copy link
Member Author

What needed fixing? Both actions runs produce identical SVGs. All three PDFs also look fine to me.

I'm wondering if this is a platform specific rendering issue? I'm using Firefox on Ubuntu to look at them and while there are some very minor whitespace differences between the CI produced assets and the one from the release, either seems fine to me and without the comparison I wouldn't notice anything amiss. (Happy to provide screenshots if that's useful)

Please could you elaborate on what the issues were which needed fixing?

@WillB97
Copy link
Contributor

WillB97 commented Mar 31, 2025

Capital A's render incorrectly.
image

@richardbarlow
Copy link
Contributor

richardbarlow commented Mar 31, 2025

That looks like the fill rule on the text object being set to even-odd: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/fill-rule

In Inkscape, changed with the buttons in the top right of the fill/stroke dialog: https://inkscape-manuals.readthedocs.io/en/latest/_images/fill_and_stroke_dialog-stroke_paint_tab.png

@PeterJCLaw
Copy link
Member Author

That is most odd, though now you mention it I feel we've maybe had this issue before? Oddly it doesn't reproduce for me, even now I know what to look for, very weird. We should definitely get this logged so we're not relying on individuals happening to know about it to fix it.

@PeterJCLaw
Copy link
Member Author

srobo/comp-floorplans#41

@PeterJCLaw
Copy link
Member Author

The competition happened.

@PeterJCLaw PeterJCLaw closed this Apr 21, 2025
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.

3 participants