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

Add virtual display python bindings #1464

Merged
merged 3 commits into from
Aug 25, 2021

Conversation

michdolan
Copy link
Collaborator

Adds missing Python bindings for OCIO Config virtual display interface.

Fixes #1317

Signed-off-by: Michael Dolan <michdolan@gmail.com>
@michdolan michdolan added this to the OCIO 1.2 milestone Aug 23, 2021
@michdolan
Copy link
Collaborator Author

@meimchu are you still interested in writing tests for these new Python bindings?

Copy link
Member

@hodoulp hodoulp left a comment

Choose a reason for hiding this comment

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

LGTM. But I think there should be at least one Python test. Even if that's platform specific you can get some hints from Config_tests.cpp OCIO_ADD_TEST(Config, virtual_display) to create something platform agnostic (and postpone the platform specific parts for now).

@hodoulp
Copy link
Member

hodoulp commented Aug 23, 2021

That's a candidate for 2.0.2

Signed-off-by: Michael Dolan <michdolan@gmail.com>
@michdolan
Copy link
Collaborator Author

Added an implementation of the platform agnostic portion of OCIO_ADD_TEST(Config, virtual_display) to the Python tests. In the process I found a few other missing methods around the virtual displays in there and added them.

@meimchu
Copy link
Contributor

meimchu commented Aug 24, 2021

Thanks @michdolan for pinging me about this. Let me take a look at it for creating python unit tests!

@michdolan
Copy link
Collaborator Author

@meimchu I reimplemented a basic test from the C++ unit tests so this can be merged, but as this is a complex feature there is more work to be done in a future PR, specifically testing the platform specific virtual display/system monitor interaction. Examples of that also live in the C++ tests. Thanks for your help!

@michdolan michdolan modified the milestones: OCIO 1.2, OCIO 2.1 Aug 24, 2021
@hodoulp hodoulp merged commit 7f74187 into AcademySoftwareFoundation:master Aug 25, 2021
@remia
Copy link
Collaborator

remia commented Feb 9, 2022

As mentioned on Slack, should we port this to the RB 2.0 branch? This is currently not available on 2.0.3 but is on 2.1.0 as @doug-walker found.

@hodoulp
Copy link
Member

hodoulp commented Feb 9, 2022

The idea around a potential 2.0.3 version is to only bring critical fixes (e.g. color processing errors like in 2.0.2) and suggest users to switch to the latest 2.1 version as soon as possible. At a technical point of view, updating from 2.0.x to 2.0.4 or 2.1.2 should be the same among of work. The point is to put extra attention not increasing too much the number of supported versions. So, I suggest you to bring the topic to the next TSC meeting to agree on a version support policy.

@remia
Copy link
Collaborator

remia commented Feb 15, 2022

Thanks @hodoulp, I'll add it to the agenda for the next meeting.

hodoulp pushed a commit to hodoulp/OpenColorIO that referenced this pull request Mar 8, 2022
* Add virtual display python bindings

Signed-off-by: Michael Dolan <michdolan@gmail.com>

* Add virtual display test, missing methods

Signed-off-by: Michael Dolan <michdolan@gmail.com>
hodoulp added a commit that referenced this pull request Mar 11, 2022
#1611)

* Add virtual display python bindings (#1464)

* Add virtual display python bindings

Signed-off-by: Michael Dolan <michdolan@gmail.com>

* Add virtual display test, missing methods

Signed-off-by: Michael Dolan <michdolan@gmail.com>

* Fix the Config Python binding

Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Co-authored-by: Michael Dolan <michdolan@gmail.com>
@michdolan michdolan deleted the py_icc_monitor branch July 26, 2023 22:19
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.

Add missing ICC monitor methods to Python binding
5 participants