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

QModelIndex::internalId() needed for trees #719

Closed
1 of 3 tasks
kermitfrog opened this issue Oct 29, 2023 · 3 comments · Fixed by #750
Closed
1 of 3 tasks

QModelIndex::internalId() needed for trees #719

kermitfrog opened this issue Oct 29, 2023 · 3 comments · Fixed by #750
Assignees
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 🙋 good first issue Good for newcomers

Comments

@kermitfrog
Copy link

kermitfrog commented Oct 29, 2023

Possible tasks:

Original description:

I am trying to create a tree-model to provide data to QML.
The normal way is to use QAbstractItemModel, but there is a problem:
The view might request an items parent by calling QModelIndex::parent().

In a list or table, where items never have parents, this would simply return an invalid QModelIndex().
The current item can then be found simply by it's row and column.

In a tree, the parent is needed as well, for which I need to know the current Item, for which I need the parent ...
That problem is usually solved by storing a pointer or id in the QModelIndex, which is set by QAbstractItemModel::createIndex() and retrieved by QModelIndex::internalId() (or internalPointer()) -- but cxx_qt does not provide access to these methods.

I thought about a workaround by following the parents up to the root level, remembering all the rows and then down again to find the right item, but as the documentation of QAbstractItemModel::parent states:

When reimplementing this function in a subclass, be careful to avoid calling QModelIndex member functions, such as QModelIndex::parent(), since indexes belonging to your model will simply call your implementation, leading to infinite recursion.

so that would not work.

The only other way I can think of, that might possibly work, is subclassing QModelIndex, which I'd like to avoid.

@ahayzen-kdab
Copy link
Collaborator

Thanks for taking the time to report this issue :-)

We haven't wrapped those methods in CXX-Qt yet, we could add support for the internalPointer quite easily by the looks of things or if added support for quintptr that would also be simple to add.

Currently you could extend the QModelIndex type with your own type, for example

You could define your own type to extend the cxx_qt_lib one.

#[repr(transparent)]
pub struct QModelIndexExtended(cxx_qt_lib::QModelIndex);

unsafe impl cxx::ExternType for QModelIndexExtended {
    type Id = cxx::type_id!("QModelIndex");
    type Kind = cxx::kind::Trivial;
}

Then in your bridge define the type with additional methods

    unsafe extern "C++" {
        include!("cxx-qt-lib/qmodelindex.h");
        type QModelIndex = super::QModelIndexExtended;

        type c_void;

        // TODO: could build quintptr as a type then this would work too
        // or create a trampoline from C++ which casts to a type CXX understands
        // #[rust_name = "internal_id"]
        // fn internalId(self: &QModelIndex) -> quintptr;

        #[rust_name = "internal_pointer"]
        unsafe fn internalPointer(self: &QModelIndex) -> *mut c_void;
    }

Then in a header that is included you would also need using c_void = void; (eg i put it in the qmodelindex.h but you could add another header and then include it).

These are all normal CXX types so follow the normal CXX rules.

But I also think that we should consider adding built in support for the c_void cast in C++ for CXX ( dtolnay/cxx#1049 ) and we should consider adding types to cxx-qt-lib for things like quintptr so that these can be easily expressed. Then we could add some of these extra methods to the built-in types.

@ahayzen-kdab ahayzen-kdab added ⬆️ feature New feature or request 🙋 good first issue Good for newcomers 🤔 discussion Feedback welcome labels Oct 30, 2023
@kermitfrog
Copy link
Author

Thanks for providing code, but unfortunetely I can not get it to compile.
I tried several things and re-read the docs (I'm new to cxx and really wanted to understand why it's not working - as far as I understand it should..), but I always end up with this error:

error: ‘c_void’ in namespace ‘::’ does not name a type; did you mean ‘void’?
warning: 286 | using c_void = ::c_void;
warning: | ^~~~~~
warning: | void

Do you have any ideas?
Otherwise I'll just wait until #724 is merged.

@ahayzen-kdab
Copy link
Collaborator

In a C++ header you need to put using c_void = void; and then include it. Something like the following might work (untested) ...

cpp/my_header.h

using c_void = void;

Then in the bridge you need to include it

#[cxx_qt::bridge]
mod ffi {
...
    unsafe extern "C++ {
        include!("my_header.h");
    }
...
}

Then you need to make sure that the header is in the C++ include path, so in the build.rs ensure that you have something that includes the cpp folder.

fn main() {
    CxxQtBuilder::new()
        ...
        .cc_builder(|cc| {
            // needs to point to the folder where the header is
            cc.include("cpp");
        })
        ...
        .build();
}

@LeonMatthesKDAB LeonMatthesKDAB self-assigned this Nov 22, 2023
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue Nov 22, 2023
Unfortunately this currently requires the use of our own c_void type.
See: dtolnay/cxx#1049

Closes KDAB#719
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue Dec 15, 2023
Unfortunately this currently requires the use of our own c_void type.
See: dtolnay/cxx#1049

Closes KDAB#719
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue Dec 15, 2023
Unfortunately this currently requires the use of our own c_void type.
See: dtolnay/cxx#1049

Closes KDAB#719
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue Dec 15, 2023
Unfortunately this currently requires the use of our own c_void type.
See: dtolnay/cxx#1049

Closes KDAB#719
ahayzen-kdab pushed a commit that referenced this issue Dec 15, 2023
Unfortunately this currently requires the use of our own c_void type.
See: dtolnay/cxx#1049

Closes #719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 🙋 good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants