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.

Sign up for GitHub

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

Jump to bottom

WIP: cxx-qt-lib: add serde support for types #425

Draft
wants to merge 31 commits into
base: main
Choose a base branch
from
Draft

WIP: cxx-qt-lib: add serde support for types #425

wants to merge 31 commits into from

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Feb 1, 2023 •
edited

Related to #292

Requires #416

@ahayzen-kdab ahayzen-kdab mentioned this pull request Feb 1, 2023
Closed
8 tasks
ahayzen-kdab
ahayzen-kdab commented Feb 7, 2023
View reviewed changes
crates/cxx-qt-lib/src/core/qdate.rs Outdated
@@ -48,6 +48,7 @@ mod ffi {

/// The QDate class provides date functions.
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[repr(C)]
pub struct QDate {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we serialise as a number or the d,m,y dictionary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes gone with this. Although what format should be used? i've used {"year":2023,"month":1,"day":1} for now but is there a standard? like should it be y,m,d ? (and same for QTime, QTimeZone, QSize eg width or w etc).

@ahayzen-kdab ahayzen-kdab force-pushed the 292-serde-support branch 2 times, most recently from 0baee32 to 36828d5 Compare February 7, 2023 07:56
ahayzen-kdab
ahayzen-kdab commented Feb 7, 2023
View reviewed changes
crates/cxx-qt-lib/src/core/qbytearray.rs
let mut bytes = QByteArray::default();
bytes.reserve(seq.size_hint().unwrap_or(0) as isize);

while let Some(value) = seq.next_element::<u8>()? {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this the best way to represent QByteArray?

@ahayzen-kdab ahayzen-kdab force-pushed the 292-serde-support branch 2 times, most recently from 3f740b2 to 81e446c Compare February 14, 2023 10:50
@LeonMatthesKDAB
Copy link
Collaborator

Side note whilst skimming this:
It seems this brings up the issue of how stable the binary representation of certain Qt classes is (i.e. QPoint).
We should really figure this out first. Does Qt guarantee, by virtue of guaranteeing binary compatibility, that e.g. the layout of QPoint can't change?

If so, we can also keep making members of these classes public, as we don't have to worry about their order changing between minor versions.

@ahayzen-kdab
Copy link
Collaborator Author

Side note whilst skimming this: It seems this brings up the issue of how stable the binary representation of certain Qt classes is (i.e. QPoint). We should really figure this out first. Does Qt guarantee, by virtue of guaranteeing binary compatibility, that e.g. the layout of QPoint can't change?

If so, we can also keep making members of these classes public, as we don't have to worry about their order changing between minor versions.

Right, I think ones like QPoint are unlikely to change, but as we've also seen while investigating QTimeZone that has changed it's layout in future Qt versions.

Eg with QSharedDataPointer https://github.com/qt/qtbase/blob/v6.4.2/src/corelib/time/qtimezone.h#L152 becoming a Data (a union with largest size of pointer) https://github.com/qt/qtbase/blob/v6.5.0-beta2/src/corelib/time/qtimezone.h#L242 so i think still the same size ?

@ahayzen-kdab ahayzen-kdab force-pushed the 292-serde-support branch 3 times, most recently from eeb00e1 to 1a84cbd Compare February 21, 2023 18:15
@ahayzen-kdab
cxx-qt-lib: add QTimeZone
cda809e 
Related to KDAB#291
@ahayzen-kdab
cxx-qt-lib: add QDateTime methods
551dda4 
Related to KDAB#55
@ahayzen-kdab
cxx-qt-lib: add methods to QStringList
11fb918 
Related to KDAB#55
@ahayzen-kdab
cmake: ensure that all features are tested in CI
7cfeb88
@ahayzen-kdab
cxx-qt-lib: add serde support for types that can derive
b651095 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support to QString
2d90c12 
Related to KDAB#292
@ahayzen-kdab
examples: use serde feature support instead of proxy
1134dc0 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QList
27fcd11 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QVector
2917ec0 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QSet
42712f7 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QByteArray
b29af03 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QStringList
5c040d9 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QUrl
a215eaa 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QHash
fd2329a 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support for QMap
193ab57 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: add serde support to QVectorND
ed62761 
Related to KDAB#292
@ahayzen-kdab
cxx-qt-lib: move serde derive to explicit creation
370ebcb 
This is so that any changes to Qt don't break serde.

Related to KDAB#292
ahayzen-kdab
ahayzen-kdab commented Feb 21, 2023
View reviewed changes
crates/cxx-qt-lib/src/gui/qcolor.rs
@@ -9,31 +9,247 @@ use std::mem::MaybeUninit;

#[cxx::bridge]
mod ffi {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to add serde support for QColor, either with all the formats as well or converting to one of them?

@ahayzen-kdab ahayzen-kdab changed the title WIP: cxx-qt-lib: add serde support for types cxx-qt-lib: add serde support for types Feb 28, 2023
@ahayzen-kdab ahayzen-kdab mentioned this pull request Mar 6, 2023
Closed
@ahayzen-kdab
Copy link
Collaborator Author

Going to pause for 0.5 and consider if we do want to maintain this. As any change in the serde format would break 1.0 release. And if the developer can easily change it to their own types with To/From do we even need to mantain this?

@ahayzen-kdab ahayzen-kdab changed the title cxx-qt-lib: add serde support for types WIP: cxx-qt-lib: add serde support for types Mar 7, 2023
@LeonMatthesKDAB LeonMatthesKDAB mentioned this pull request Apr 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ahayzen-kdab @LeonMatthesKDAB