Skip to content

feat: add QQmlImageProviderBase#1430

Open
dphaldes wants to merge 6 commits intoKDAB:mainfrom
dphaldes:feat/QQmlImageProvider
Open

feat: add QQmlImageProviderBase#1430
dphaldes wants to merge 6 commits intoKDAB:mainfrom
dphaldes:feat/QQmlImageProvider

Conversation

@dphaldes
Copy link
Copy Markdown
Contributor

@dphaldes dphaldes commented Mar 10, 2026

supercedes #1142

Co-authored-by: Joshua Goins <joshua.goins@kdab.com>
@dphaldes

This comment was marked as outdated.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ee1849d) to head (e66959c).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1430   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           75        75           
  Lines        13457     13455    -2     
=========================================
- Hits         13457     13455    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dphaldes

This comment was marked as outdated.

@dphaldes dphaldes marked this pull request as ready for review March 20, 2026 01:58
@dphaldes
Copy link
Copy Markdown
Contributor Author

I have "working" implementation here https://github.com/dphaldes/compose/blob/master/src/bridge.rs.
image

Not particularly fond of the solution since it uses an unchecked cast since I wasn't able to figure out how to cast Pin<&mut QQmlImageProviderBase> into *mut QQmlImageProviderBase. The application also segfaults when closing probably because it is trying to release the ImageProvider.

Any ideas appreciated!

@ahayzen-kdab
Copy link
Copy Markdown
Collaborator

How are you making your QQmlImageProviderBase implementation? As looks like you have a UnqiuePtr<PreviewProvider> which you can do as_mut_ptr() or into_inner() to get a *mut PreviewProvider which then could be cast to a *mut QQmlImageProviderBase ? Or do you need to do it in a place where you only have a Pin ?

And generally things here are looking like they are heading in the right direction :-)

https://docs.rs/cxx/latest/cxx/struct.UniquePtr.html#method.as_mut_ptr

@dphaldes
Copy link
Copy Markdown
Contributor Author

I didn't see that method. I will look into using that. It would work but it still wouldnt work as a safe cast will it? as in using the Upcast trait instead of as keyword

@ahayzen-kdab
Copy link
Copy Markdown
Collaborator

Hmm right we don't have any safe API for casting pointers, there is upcast_ptr (basically static_cast) and from_base_ptr (basically dynamic_cast) https://github.com/KDAB/cxx-qt/blob/main/crates/cxx-qt/src/casting.rs#L23 ... but both of these are *const T not *mut T.

I think for now just use an as or the cast_mut() from std https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.cast_mut Raw pointers are always going to be tricky with Rust, so we should consider whether we can improve any help there or if writing .cast_mut() is fine

@dphaldes
Copy link
Copy Markdown
Contributor Author

That works pretty well on the implementation side. https://github.com/dphaldes/compose/blob/38019867036cce5d156e786884567c565fdd7903/src/main.rs#L46-L51

Had to use into_raw() so that we release the UniquePtr early and avoid the segfault when QmlEngine takes ownership of the Provider.
Didnt require anything changed from cxx-qt side. So this PR should be ready for review and cleanup

Comment on lines +78 to +86
#[rust_name = "add_image_provider"]
unsafe fn addImageProvider(
self: Pin<&mut QQmlApplicationEngine>,
provider_id: &QString,
provider: *mut QQmlImageProviderBase,
);

#[rust_name = "remove_image_provider"]
fn removeImageProvider(self: Pin<&mut QQmlApplicationEngine>, provider_id: &QString);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

these should be on the QQmlEngine rather than the QQmlApplicationEngine (which inherits from the QQmlEngine)

Pixmap,
Texture,
ImageResponse,
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In Qt 5 these enums are slightly different either have a cfg like #[cfg(cxxqt_qt_version_major = "6")] and #[cfg(cxxqt_qt_version_major = "5")] to have different enums or disable all of this for Qt 5.

In Qt 5

In Qt 6

Note in Qt 5 there is a gap of no 3 :-) And do we need the Invalid = 0 ? As that is not defined on the Qt side ?

Comment on lines +23 to +24
#[qobject]
type QQmlImageProviderBase;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In Qt 5, this is just a normal class and does no inherit from QObject, so again might need a cfg block to have two different types if we want to make it work with Qt 5


#[cxx_qt::bridge]
mod ffi {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remove empty line :-)

@ahayzen-kdab
Copy link
Copy Markdown
Collaborator

Looking overall good, just need to either decide to fix the Qt 5 support or opt-out for these methods/types

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