Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Loader UI: Fix right click in representation widget #2757

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

iLLiCiTiT
Copy link
Member

Brief description

This PR broke right click of representation widget. It also used ItemRole which would not work with older python + PySide.

Description

Don't use ItemRole in loader since it won't pass right object in older Python + PySide combination because proxy model will convert it to known python oject which cause lost of methods/attributes.

Changes

  • use item id in representations widget

Testing notes:

Site sync module must be enabled

  1. Open loader in host
  2. Right click on representation in representation widget

@iLLiCiTiT iLLiCiTiT self-assigned this Feb 18, 2022
@iLLiCiTiT iLLiCiTiT added the type: bug Something isn't working label Feb 18, 2022
@BigRoy
Copy link
Collaborator

BigRoy commented Feb 18, 2022

Don't use ItemRole in loader since it won't pass right object in older Python + PySide combination because proxy model will convert it to known python oject which cause lost of methods/attributes.

Where could I detect specifically this problem? Since the problem also seems to exist in e.g. Maya 2020 I'd assume it's not 'older PySide' unless you're referring to older PySide2 with Python 2?

@mkolar
Copy link
Member

mkolar commented Feb 18, 2022

Works for me now. @BigRoy the problem is visible in loader. When you right click on a representation, you don't get any context menu with loaders at all without the fix

@BigRoy
Copy link
Collaborator

BigRoy commented Feb 18, 2022

When you right click on a representation, you don't get any context menu with loaders at all without the fix

I understand the bug - but I'm not sure the way it's fixed or at least what's described here to be the issue to be correct. I've not seen before that Qt would convert the type of the returned object and thus it breaking.

This sounds more like an issue with the Proxy Model than of Qt itself. I've seen that happen with proxy models if they weren't implemented correctly to pass in the right data. It seems like currently we're trying to work around the model to actually return the data.

because proxy model will convert it to known python oject which cause lost of methods/attributes.

Could you point to me which proxy model is involved breaking this scenario? :)

@mkolar
Copy link
Member

mkolar commented Feb 18, 2022

I'll merge this because it's actually making any work really difficult without it, however, @iLLiCiTiT have a look at Roy's comments please afterwards.

@mkolar mkolar merged commit 40fcfdb into develop Feb 18, 2022
@mkolar mkolar deleted the bugfix/context_menu_in_representation_widget branch February 18, 2022 14:39
@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Feb 19, 2022

This sounds more like an issue with the Proxy Model than of Qt itself.

It is partially true but there's not much we can do about it. It happens in dcc which have older Qt binding or with small tweaks in c++. Nuke had this issue I think up to v 10, it seems that flame has similar issue.

Accessing data through roles is not meant to return complex objects but QVariant (Documentation: "acts like a union for the most common Qt data types"). It seems that older versions are trying to convert the value into python common data types which for Item class is dict thus this happens. I've tried overridde proxy model methods to avoid this but it fixed only few cases and at the end it just add complexity into parts of code where it should not matter. Just to be able get value which is not subset of QVariant types...

I don't think this fix is ideal but accessing Item object is not either.

EDITED:
This PR did 2 changes.

  1. fixed removed methods done during this PR - my mistake
  2. changed usage of ItemRole to fix the tool in flame because it would not work there

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants