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

Pattern pieces grouped in exported SVGs #1195

Merged
merged 7 commits into from
Oct 8, 2024
Merged

Pattern pieces grouped in exported SVGs #1195

merged 7 commits into from
Oct 8, 2024

Conversation

Onetchou
Copy link
Contributor

When a SVG is generated from piece or layout mode, each pattern piece is in its own group.

To achieve that, I decided to manually modify the generated xml files manipulating the corresponding DOM trees, because Qt doesn't allow to add groups into exported SVG files.

When a user exports a SVG named foo.svg, the software will create a tempSvg_foo directory into the destination directory, create a SVG (tempSVG_foo_xx.svg) for each pattern piece to be exported, and then merge all the SVG together to group them separately. After the process, all the temporary files and folders are automatically deleted.

I'm not super happy about the software creating temporary files into the destination directory though... is there another (better) place where I could generate those temporary files ? @DSCaskey

Copy link

what-the-diff bot commented Sep 22, 2024

PR Summary 📚

Hooray! 👏 We've made some pretty exciting updates to our SVG generation capabilities. Let's dive in and break down how they make our system even better.

  • Created a New 'SvgGenerator' Class 🆕
    Meet our new team member, the 'SvgGenerator' class. It's dedicated to creating and combining SVG files. This means we now have a neat little toolbox focused solely on handling SVGs. This keeps our code more organized and our developers happier. 😊

  • Refreshed the exportSVG Method ⚙️
    We've retooled how the exportSVG method in our MainWindowsNoGUI works. Now, it'll be using our new 'SvgGenerator' class to create SVG files. This shift means more streamlined, efficient SVG file generation. The exportSVG method just got a superpower! 🦸

  • Extended exportSVG Method's Abilities 🔄
    Our exportSVG method is now more versatile! It's got a new version that can accept a list of QGraphicsItem objects to turn into SVGs. Having this additional functionality just means more flexibility for our developers. 🤸

  • Created New Files for Better Organization 📁
    We've introduced two new files, svg_generator.cpp and svg_generator.h, which will home our new 'SvgGenerator' class and its operations. This helps keep our code clean and our developers focused. It's like giving our SvgGenerator its own personal suite. 🏡

  • Updated Paths for Seamless Integration 🕸️
    We've integrated the new svg_generator.h into mainwindowsnogui.cpp and updated the corresponding files to connect seamlessly with our new SVG creation infocenter. Meaning, all related files now know where and how to access SvgGenerator. It's like updating their GPS for smooth travelling! 🌍

  • Reworked SVG Export Flow for Improved Operations 🌊
    We've remodelled the SVG export process in the ExportScene function to accommodate our new exportSVG method version. This revamp ensures the modified SVG exportation flows smoothly in the code river.

Let's celebrate these fantastic updates and keep pushing for excellence. Onwards and upwards, team! 🎉🚀

@DSCaskey
Copy link
Contributor

DSCaskey commented Sep 24, 2024

I'm not super happy about the software creating temporary files into the destination directory though... is there another (better) place where I could generate those temporary files

QString QDir::tempPath()
Returns the absolute canonical path of the system's temporary directory.

QDir QDir::temp()
Returns the system's temporary directory.

Copy link
Contributor

@DSCaskey DSCaskey left a comment

Choose a reason for hiding this comment

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

Looks great. Can you please add comments for the SvgGenerator class & methods so that they are included in the Doxygen docs during the CI?

As an example I'd like to stick to this format:

// @brief Adds a new piece to the container.
//
// This method adds a new piece to the container and returns its ID.
//
// @param piece The piece to add to the container.
// @return quint32 The ID of the new piece in the container.
//
// @details
// - The method generates a new unique ID for the piece by calling getNextId().
// - It inserts the piece into the internal hash map of pieces using the generated ID.
// - The method returns the ID assigned to the new piece.

quint32 VContainer::AddPiece(const VPiece &piece)

@Onetchou
Copy link
Contributor Author

Onetchou commented Sep 24, 2024

I'm not super happy about the software creating temporary files into the destination directory though... is there another (better) place where I could generate those temporary files

QString QDir::tempPath() Returns the absolute canonical path of the system's temporary directory.

QDir QDir::temp() Returns the system's temporary directory.

I feel so dumb right now 😂
I never had to write temporary files before, and for some reasons I didn't even think about using the system temp directory...

Btw I found a way not to write any temporary files at all lmao, I'll add it to the PR when I have time 😊 (as a memo for myself : I need to use a QBuffer as the output device of the qSvgGenerator, I can then have the SVGs directly into bytearrays and convert them easily into QStrings and / or their corresponding DOM trees)

@Onetchou
Copy link
Contributor Author

Looks great. Can you please add comments for the SvgGenerator class & methods so that they are included in the Doxygen docs during the CI?

As an example I'd like to stick to this format:

// @brief Adds a new piece to the container.
//
// This method adds a new piece to the container and returns its ID.
//
// @param piece The piece to add to the container.
// @return quint32 The ID of the new piece in the container.
//
// @details
// - The method generates a new unique ID for the piece by calling getNextId().
// - It inserts the piece into the internal hash map of pieces using the generated ID.
// - The method returns the ID assigned to the new piece.
quint32 VContainer::AddPiece(const VPiece &piece)

Of course !

@Onetchou
Copy link
Contributor Author

Fixes #716

@Onetchou
Copy link
Contributor Author

Done.

I added comments and I simplified the whole class by not creating temporary files anymore.

@Onetchou Onetchou requested a review from DSCaskey September 28, 2024 10:48
Copy link
Contributor

@DSCaskey DSCaskey left a comment

Choose a reason for hiding this comment

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

For Doxygen to include the @Briefs (as formated) , comment blocks need to go before the Method / function / member declaration.

There's a way to format the comments when positioned after, but it's breaking with the way we're currently placing the comment blocks.

Also since a new translation has been added to the ui, an lupdate should be run.

@Onetchou
Copy link
Contributor Author

Onetchou commented Sep 29, 2024

For Doxygen to include the @Briefs (as formated) , comment blocks need to go before the Method / function / member declaration.

There's a way to format the comments when positioned after, but it's breaking with the way we're currently placing the comment blocks.

OK, I'll change that

Also since a new translation has been added to the ui, an lupdate should be run.

👌

@DSCaskey DSCaskey merged commit 000cbc3 into develop Oct 8, 2024
10 checks passed
@DSCaskey DSCaskey deleted the svg-groups branch October 8, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants