search

AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.76k stars 434 forks source link

Improve ocioview mac support and simplify dependencies #1853

Closed remia closed 10 months ago

remia commented 11 months ago

This is the branch I shared earlier with some fixes to make it launch on my version of mac OS. I'll leave that in Draft for the time being as there are still some rough edges and I haven't confirmed if this is due to my changes or just the current development state.

To comment on last week TSC discussion, adding a "bookmark" with Ctrl+Number then toggling the current output at the top of the viewer does refresh the image correctly for me.

Note that I originally tried to run it with PySide2 but got stuck with errors like this one and gave up after some time as it seemed easier to just migrate on version 6 which just works out of the box.

Traceback (most recent call last):
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/main.py", line 13, in <module>
    from ocioview.main_window import OCIOView
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/main_window.py", line 13, in <module>
    from .config_dock import ConfigDock
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/config_dock.py", line 9, in <module>
    from .items import (
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/items/__init__.py", line 4, in <module>
    from .color_space_edit import ColorSpaceEdit
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/items/color_space_edit.py", line 12, in <module>
    from ..widgets import (
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/widgets/__init__.py", line 15, in <module>
    from .list_widget import StringListWidget, ItemModelListWidget
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/widgets/list_widget.py", line 10, in <module>
    from .item_view import BaseItemView
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/widgets/item_view.py", line 14, in <module>
    class BaseItemView(QtWidgets.QFrame):
  File "/Users/remi/ColorCode/OpenColorIO/src/apps/ocioview/ocioview/widgets/item_view.py", line 24, in BaseItemView
    QtCore.Qt.ItemIsEnabled | QtCore.Qt.ItemIsEditable | QtCore.Qt.ItemIsSelectable
                                                         ^^^^^^
TypeError: 'PySide2.QtCore.Qt.ItemFlag' object cannot be interpreted as an integer
KelSolaar commented 11 months ago

Moving to PySide 6 makes sense anyway, the VFX Reference Platform 2024 is on Qt 6.

michdolan commented 11 months ago

as there are still some rough edges and I haven't confirmed if this is due to my changes or just the current development state.

Any specific issues you are seeing?

linux-foundation-easycla[bot] commented 11 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

remia commented 11 months ago

Any specific issues you are seeing?

I took a look again today, using this branch (on a Windows workstation though). I realised I broke the pixel probe by adding the imageio support so fixed that in the new commit. I still see an issue with bookmarks switching: when assigning views from different displays (eg. ACES SDR from sRGB and Rec.2100), the switch don't work unless the display you want to switch to is selected in the right panel, hopefully that make sense? Otherwise it seems to work pretty well.

michdolan commented 10 months ago

There are some bookmark changes in PR #1845 which may resolve the issue you are seeing. See:

https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1845/files#diff-174a130d87ad127f271fc81ffed6fc91ee3fc7237203d4ac6ba63233149b1c07

https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1845/files#diff-b1c07afbe47f976c8df81907f57689abb516a7ea119714bae093bb950ea47eb7

KelSolaar commented 10 months ago

The branch has been generally working for me, I think it would be useful to merge it so that we can move forward. We can fix problems as they come!

remia commented 10 months ago

This should be ready to merge, sorry about rebasing it may reset the review status of Github unfortunately.

KelSolaar commented 10 months ago

Nice!