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.

Sign up for GitHub

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

Publishing reviewable attributes enhancements #24

Merged
merged 14 commits into from
Nov 12, 2024
Merged

Publishing reviewable attributes enhancements #24

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e3342ec
Improving reviewable UX on instances
jakubjezek001 Nov 6, 2024
0237f09
collect reviewable flags correctly.
jakubjezek001 Nov 6, 2024
9461c2d
Update client/ayon_hiero/plugins/create/create_shot_clip.py
jakubjezek001 Nov 7, 2024
62f65d3
WIP on (no branch)
jakubjezek001 Nov 7, 2024
0200cda
Update review-related attributes and visibility in plugin code. Fix a…
jakubjezek001 Nov 7, 2024
8cbf30c
turning reviewable track into reviewable source
jakubjezek001 Nov 8, 2024
c567f53
fixing plate attributes
jakubjezek001 Nov 8, 2024
e3c4acc
Refactor review visibility logic in creator attributes
jakubjezek001 Nov 8, 2024
a81eb8f
Refactor code to simplify setting visibility of reviewable source.
jakubjezek001 Nov 8, 2024
7c69d92
Fix comparison operator for review source in PublishClip class
jakubjezek001 Nov 8, 2024
7d4323b
Update source selection tooltip for reviewable files
jakubjezek001 Nov 8, 2024
b60aa07
Merge branch 'develop' into enhancement/AY-7104_reviewable-flag-distr…
jakubjezek001 Nov 12, 2024
cc94470
Add backward compatibility for reviewableSource (2024.11.08)
jakubjezek001 Nov 12, 2024
32799aa
Update reviewable source visibility and disable state based on review…
jakubjezek001 Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions client/ayon_hiero/api/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ class PublishClip:
hierarchy_default = "{_folder_}/{_sequence_}/{_track_}"
clip_name_default = "shot_{_trackIndex_:0>3}_{_clipIndex_:0>4}"
product_name_default = "<track_name>"
review_track_default = "< none >"
review_source_default = None
product_type_default = "plate"
count_from_default = 10
count_steps_default = 10
Expand All @@ -689,7 +689,9 @@ class PublishClip:
# publish settings
"audio", "sourceResolution",
# shot attributes
"workfileFrameStart", "handleStart", "handleEnd"
"workfileFrameStart", "handleStart", "handleEnd",
# instance attributes data
"reviewableSource",
}

def __init__(
Expand Down Expand Up @@ -743,8 +745,9 @@ def convert(self):

# if track name is in review track name and also if driving track name
# is not in review track name: skip tag creation
if (self.track_name in self.review_layer) and (
self.driving_layer not in self.review_layer):
if (self.track_name in self.reviewable_source) and (
self.driving_layer not in self.reviewable_source
):
return

# deal with clip name
Expand All @@ -765,11 +768,6 @@ def convert(self):
)
self.tag_data["folderPath"] = folder_path

if self.tag_data["heroTrack"] and self.review_layer:
self.tag_data.update({"reviewTrack": self.review_layer})
else:
self.tag_data.update({"reviewTrack": None})

return self.track_item

def _populate_track_item_default_data(self):
Expand All @@ -795,7 +793,7 @@ def _populate_attributes(self):
log.debug(
"____ self.shot_num: {}".format(self.shot_num))

# publisher ui attribute inputs or default values if gui was not used
# publisher ui attribute inputs or default values if gui was not used
def get(key):
"""Shorthand access for code readability"""
return self.pre_create_data.get(key)
Expand All @@ -810,7 +808,8 @@ def get(key):
self.product_type = get("productType") or self.product_type_default
self.vertical_sync = get("vSyncOn") or self.vertical_sync_default
self.driving_layer = get("vSyncTrack") or self.driving_layer_default
self.review_track = get("reviewTrack") or self.review_track_default
self.review_source = (
get("reviewableSource") or self.review_source_default)
self.audio = get("audio") or False

self.hierarchy_data = {
Expand Down Expand Up @@ -841,7 +840,7 @@ def _convert_to_tag_data(self):
"""
# define vertical sync attributes
hero_track = True
self.review_layer = ""
self.reviewable_source = ""
if self.vertical_sync:
# check if track name is not in driving layer
if self.track_name not in self.driving_layer:
Expand All @@ -857,16 +856,24 @@ def _convert_to_tag_data(self):

# adding tag metadata from ui
for _key, _value in self.pre_create_data.items():
if (
_key == "reviewableSource"
and "reviewTrack" in self.tag_keys
):
self.tag_data.pop("reviewTrack")
self.tag_data["reviewableSource"] = _value
if _key in self.tag_keys:
# backward compatibility for reviewableSource (2024.11.08)
jakubjezek001 marked this conversation as resolved.
Show resolved Hide resolved

self.tag_data[_key] = _value

# driving layer is set as positive match
if hero_track or self.vertical_sync:
# mark review layer
if self.review_track and (
self.review_track not in self.review_track_default):
if self.review_source and (
self.review_source != self.review_source_default):
# if review layer is defined and not the same as default
self.review_layer = self.review_track
self.reviewable_source = self.review_source
# shot num calculate
if self.rename_index == 0:
self.shot_num = self.count_from
Expand Down Expand Up @@ -924,11 +931,30 @@ def _convert_to_tag_data(self):
self.tag_data["uuid"] = str(uuid.uuid4())

# add review track only to hero track
if hero_track and self.review_layer:
self.tag_data["reviewTrack"] = self.review_layer
if hero_track and self.reviewable_source:
self.tag_data["reviewTrack"] = self.reviewable_source
else:
self.tag_data.update({"reviewTrack": None})

# add only review related data if reviewable source is set
if self.reviewable_source:
review_switch = True
reviewable_source = self.reviewable_source
#
if self.vertical_sync and not hero_track:
review_switch = False
reviewable_source = False

if review_switch:
self.tag_data["review"] = True
else:
self.tag_data.pop("review", None)

if reviewable_source:
self.tag_data["reviewableSource"] = reviewable_source
else:
self.tag_data.pop("reviewableSource", None)

def _solve_tag_hierarchy_data(self, hierarchy_formatting_data):
""" Solve tag data from hierarchy data and templates. """
# fill up clip name and hierarchy keys
Expand Down
161 changes: 113 additions & 48 deletions client/ayon_hiero/plugins/create/create_shot_clip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,43 @@ class _HieroInstanceClipCreatorBase(_HieroInstanceCreator):
""" Base clip product creator.
"""

def get_instance_attr_defs(self):
def register_callbacks(self):
self.create_context.add_value_changed_callback(self._on_value_change)

def _on_value_change(self, event):
for item in event["changes"]:
instance = item["instance"]
if (
instance is None
or instance.creator_identifier != self.identifier
):
continue

changes = item["changes"].get("creator_attributes", {})

attr_defs = instance.creator_attributes.attr_defs

if "review" not in changes:
continue

review_value = changes["review"]
reviewable_source = next(
attr_def
for attr_def in attr_defs
if attr_def.key == "reviewableSource"
)
reviewable_source.visible = review_value

instance.set_create_attr_defs(attr_defs)

def get_attr_defs_for_instance(self, instance):

current_sequence = lib.get_current_sequence()
if current_sequence is not None:
gui_tracks = [tr.name() for tr in current_sequence.videoTracks()]
gui_tracks = [
{"value": tr.name(), "label": f"Track: {tr.name()}"}
for tr in current_sequence.videoTracks()
]
else:
gui_tracks = []

Expand All @@ -192,23 +224,36 @@ def get_instance_attr_defs(self):
)
]
if self.product_type == "plate":
instance_attributes.extend([
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be sure, this means we do not want to let the user edit vSyncOn and vSyncTrack on the instance anymore ?
I think I still do not get how exactly those 2 features are connected. The way I understood it was:

  • reviewableSource + review : control if a review h264 product gets created through OIIO and FFmpeg along the plate and from which source
  • vSyncOn + vSyncTrack: define the hero track (main track) to construct a vertical synchronization.. not exactly sure what come after that

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this is quite confusing I agree:

vSync attributes does not need to be user editable on instances. Those changes were not propagated anywhere and also this would be quite difficult to prapagate them in first place. It is much more easy if user will recreate those instances from scratch.

Reviewable flaging is now doing following. Review attribute is saying if the plate needs to have acompanied reviwable representation and reviableSource is only saying what should be used for the representation creation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also very picky, but for UI/UX consistency, should we replicate the same UI between create attributes and instance attributes selection ? Currently we have 1 combo list with < none > entry in the creator then 1 checkbox + 1 combolist in the instance UI.

This is intentional. It would not make sense to have acctive review attribute with reviewableSource set to '< none >`.

BoolDef(
"vSyncOn",
label="Enable Vertical Sync",
tooltip="Switch on if you want clips above "
"each other to share its attributes",
default=True,
),
EnumDef(
"vSyncTrack",
label="Hero Track",
tooltip="Select driving track name which should "
"be mastering all others",
items=gui_tracks or ["<nothing to select>"],
),
])

# Review track visibility
current_review = instance.creator_attributes.get("review", False)

instance_attributes.extend(
[
BoolDef(
"review",
label="Review",
tooltip="Switch to reviewable instance",
default=False,
),
EnumDef(
"reviewableSource",
label="Reviewable Source",
tooltip=(
"Selecting source for reviewable files."
),
items=(
[
{
"value": "clip_media",
"label": "[ Clip's media ]",
},
]
+ gui_tracks
),
visible=current_review,
),
]
)
return instance_attributes


Expand Down Expand Up @@ -271,7 +316,10 @@ def header_label(text):

current_sequence = lib.get_current_sequence()
if current_sequence is not None:
gui_tracks = [tr.name() for tr in current_sequence.videoTracks()]
gui_tracks = [
{"value": tr.name(), "label": f"Track: {tr.name()}"}
for tr in current_sequence.videoTracks()
]
else:
gui_tracks = []

Expand Down Expand Up @@ -379,9 +427,12 @@ def header_label(text):
label="Hero track",
tooltip="Select driving track name which should "
"be mastering all others",
items=gui_tracks or ["<nothing to select>"],
items=(
gui_tracks or [
{"value": None, "label": "< nothing to select> "}
]
),
),

# publishSettings
UILabelDef(
label=header_label("Publish Settings")
Expand All @@ -401,11 +452,15 @@ def header_label(text):
items=['plate', 'take'],
),
EnumDef(
"reviewTrack",
label="Use Review Track",
"reviewableSource",
label="Reviewable Source",
tooltip="Generate preview videos on fly, if "
"'< none >' is defined nothing will be generated.",
items=['< none >'] + gui_tracks,
"'< none >' is defined nothing will be generated.",
items=[
{"value": None, "label": "< none >"},
{"value": "clip_media", "label": "[ Clip's media ]"},
]
+ gui_tracks,
),
BoolDef(
"export_audio",
Expand All @@ -416,10 +471,9 @@ def header_label(text):
BoolDef(
"sourceResolution",
label="Source resolution",
tooltip="Is resloution taken from timeline or source?",
tooltip="Is resolution taken from timeline or source?",
default=False,
),

# shotAttr
UILabelDef(
label=header_label("Shot Attributes"),
Expand Down Expand Up @@ -564,16 +618,23 @@ def create(self, subset_name, instance_data, pre_create_data):
# metadata recollection as publish time.
else:
parenting_data = clip_instances[shot_creator_id]
sub_instance_data.update({
"parent_instance_id": parenting_data["instance_id"],
"label": (
f"{shot_folder_path} "
f"{creator.product_type}"
),
"creator_attributes": {
"parentInstance": parenting_data["label"],
sub_instance_data.update(
{
"parent_instance_id": parenting_data["instance_id"],
"label": (f"{shot_folder_path} " f"{creator.product_type}"),
"creator_attributes": {
"parentInstance": parenting_data["label"]
},
}
})
)

# add reviewable source to plate if shot has it
if sub_instance_data.get("reviewableSource"):
sub_instance_data["creator_attributes"].update({
"reviewableSource": sub_instance_data[
"reviewableSource"],
"review": True,
})

instance = creator.create(sub_instance_data, None)
instance.transient_data["track_item"] = track_item
Expand Down Expand Up @@ -642,7 +703,7 @@ def _collect_legacy_instance(self, track_item):
"tag.handleStart": ("handleStart", int),
"tag.handleEnd": ("handleEnd", int),
"tag.folderPath": ("folderPath", str),
"tag.reviewTrack": ("reviewTrack", str),
"tag.reviewTrack": ("reviewableSource", str),
"tag.variant": ("variant", str),
"tag.workfileFrameStart": ("workfileFrameStart", int),
"tag.sourceResolution": ("sourceResolution", bool),
Expand Down Expand Up @@ -738,17 +799,21 @@ def _collect_legacy_instance(self, track_item):
for sub_creator_id in sub_creators:
sub_instance_data = instance_data.copy()
creator = self.create_context.creators[sub_creator_id]
sub_instance_data.update({
"clip_variant": sub_instance_data["variant"],
"parent_instance_id": parenting_data["instance_id"],
"label": (
f"{sub_instance_data['folderPath']} "
f"{creator.product_type}"
),
"creator_attributes": {
"parentInstance": parenting_data["label"],
sub_instance_data.update(
{
"clip_variant": sub_instance_data["variant"],
"parent_instance_id": parenting_data["instance_id"],
"label": (
f"{sub_instance_data['folderPath']} "
f"{creator.product_type}"
),
"creator_attributes": {
"parentInstance": parenting_data["label"],
"reviewableSource": sub_instance_data["reviewableSource"],
"review": False,
},
}
})
)

instance = creator.create(sub_instance_data, None)
instance.transient_data["track_item"] = track_item
Expand Down
16 changes: 16 additions & 0 deletions client/ayon_hiero/plugins/publish/collect_plates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ def process(self, instance):
"""
instance.data["families"].append("clip")

# solve reviewable options
review_switch = instance.data["creator_attributes"].get(
"review")
reviewable_source = instance.data["creator_attributes"].get(
"reviewableSource")

if review_switch is True:
if reviewable_source == "clip_media":
instance.data["families"].append("review")
else:
instance.data["reviewTrack"] = reviewable_source

# remove review keys from instance data
instance.data.pop("reviewableSource", None)
instance.data.pop("review", None)

# Retrieve instance data from parent instance shot instance.
parent_instance_id = instance.data["parent_instance_id"]
edit_shared_data = instance.context.data["editorialSharedData"]
Expand Down