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

Allow zones to be edited #186

Merged
merged 32 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
08c3d23
Move the trash button logic a bit to support addition of a new edit b…
BryceBeagle Aug 4, 2021
3cc13cc
Add edit button (no functionality)
BryceBeagle Aug 4, 2021
d116ec6
Merge branch 'main' into feature/181-edit-regions
BryceBeagle Aug 9, 2021
53e0110
Rename functions to better represent coming changes
BryceBeagle Aug 9, 2021
3411e68
Start using a new domain-specific object to store zone information cl…
BryceBeagle Aug 9, 2021
514c841
Move new line/region signals out of UI file
BryceBeagle Aug 10, 2021
c707848
Rename new line/region buttons to better reflect purpose
BryceBeagle Aug 10, 2021
22cf30b
Code cleanup
BryceBeagle Aug 10, 2021
fa67b8b
Start setting up signals from ZoneList
BryceBeagle Aug 10, 2021
f3da218
Use a domain layer for Zone types/data
BryceBeagle Aug 10, 2021
ba89e78
Zone vertices can now be changed
BryceBeagle Aug 11, 2021
efc7a0c
Start editing from existing zone
BryceBeagle Aug 11, 2021
c4af0c8
Fix zone edit cancelling
BryceBeagle Aug 11, 2021
eab5534
Start shifting widgets to a dedicated directory
BryceBeagle Aug 11, 2021
382302e
Get rid of zone_list.ui
BryceBeagle Aug 11, 2021
6a01c0c
Move zone_list.py to widgets/
BryceBeagle Aug 11, 2021
5753762
Move signals/slots out of task_configuration.ui
BryceBeagle Aug 11, 2021
75b1ca7
Move video_task_config.py to widgets/
BryceBeagle Aug 11, 2021
01059c5
Move in_progress_zone_item.py to the directory for this specific feature
BryceBeagle Aug 11, 2021
543f26a
Make the InProgressZoneItems use the domain Zone
BryceBeagle Aug 16, 2021
e3a894e
Use dotted line for preview zone again
BryceBeagle Aug 16, 2021
dfd98fd
Support re-edits
BryceBeagle Aug 17, 2021
e2c919e
Add edit zone SVG
velovix Aug 17, 2021
7f342a0
Rename edit_zone.svg -> edit_pencil.svg
BryceBeagle Aug 17, 2021
a53c08c
Allow editing zones after adding them
BryceBeagle Aug 17, 2021
d5bd3d2
Revert accidental debug change
BryceBeagle Aug 17, 2021
6053c73
Remove accidental line duplication
BryceBeagle Aug 17, 2021
4107f24
Adjust error messages in Zone logic
BryceBeagle Aug 17, 2021
895b614
Disable zone edit-by-addition for now
BryceBeagle Aug 17, 2021
9002b1e
Fix issue with editing existing zones caused by temp workaround
BryceBeagle Aug 18, 2021
b10516f
Improve a comment
BryceBeagle Aug 23, 2021
f7ddc1a
Always put full-frame zone at top of Zone list
BryceBeagle Aug 23, 2021
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
154 changes: 154 additions & 0 deletions brainframe_qt/ui/dialogs/task_configuration/core/zone.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
from abc import ABC, abstractmethod
apockill marked this conversation as resolved.
Show resolved Hide resolved
from dataclasses import dataclass, field
Copy link
Member

Choose a reason for hiding this comment

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

image
I believe that previously, the Screen was always first in the list. I think if it's no big deal to add, it might be nice to keep that 'feature'.

Copy link
Member Author

Choose a reason for hiding this comment

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

Umm weird... I didn't think I changed anything that would affect ordering here. I also haven't seen Screen anywhere but the top. I'll investigate

Copy link
Member Author

Choose a reason for hiding this comment

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

So I think the only way this could happen is if api.get_zones returns Screen not as the first Zone. Do you know why that would happen?

Copy link
Member

Choose a reason for hiding this comment

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

I tested it, and it appears that api.get_zones() returns Screen first, but api.get_zones(stream_id=...) returns Screen last (or maybe everything just in inverse order?).

I only have two streams of sample size, but hey.

Since this is more of a UI thing, we probably don't want to rely on the server to maintain this visual nicety. We could add this after the api.get_zones() call, to ensure the Screen zone is always first:

zones = list(map(Zone.from_api_zone, api_zones))

# These lines
screen_index = next(i for i, z in enumerate(zones) if z.name == FULL_FRAME_ZONE_NAME)
zones.insert(0, zones.pop(screen_index))

Copy link
Member

Choose a reason for hiding this comment

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

Anyways, I approved the PR, but this would be a visual nicety to have. It makes sense to have the "always persistent" uneditable zone at the top of the list always.

Copy link
Member Author

Choose a reason for hiding this comment

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

Has this functionality changed during this PR? I'm inclined to open an issue and change this in a different changeset. Also, the backend should probably be changed to have consistent ordering.

Copy link
Member

Choose a reason for hiding this comment

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

Well, yeah, it changed in the sense that now Screen is always at the bottom as opposed to always being at the top.

I consider it a minor UI regression, because I think most brainframe users have gotten used to the screen being right at the top

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be resolved now

from typing import List, Optional, Tuple, TypeVar

from brainframe.api import bf_codecs
from shapely import geometry

T = TypeVar("T")


@dataclass
class Zone(ABC):
apockill marked this conversation as resolved.
Show resolved Hide resolved
coords: Optional[List[Tuple[int, int]]]
"""Coordinates of the endpoints/vertices of the Zone. Top left of frame is (0, 0)

The full-frame region has no coords
"""

name: Optional[str] = None
"""The name of the Zone, if it has one"""

id: Optional[int] = None
"""The id of the Zone known by the BrainFrame backend"""

alarms: List[bf_codecs.ZoneAlarm] = field(default_factory=list)
"""A list of Alarms that the Zone holds"""

@abstractmethod
def copy(self: T) -> T:
"""Deep copy"""

@abstractmethod
def is_shape_ready(self) -> bool:
"""Whether this type of Zone has enough points to be considered complete"""

@abstractmethod
def takes_additional_points(self) -> bool:
"""Whether more points can be added to this type of Zone"""

@abstractmethod
def would_be_valid_zone(self, new_vertex: Tuple[int, int]) -> bool:
"""Whether the addition of new_vertex to the zone would result in a valid
zone"""

def add_vertex(self, vertex: Tuple[int, int]) -> None:
if self.coords is None:
message = (
"Zone.coords is None. Most likely attempting to add vertex to "
"full-frame Zone."
)
raise RuntimeError(message)

assert self.coords is not None
self.coords.append(vertex)

def adjust_final_vertex(self, vertex: Tuple[int, int]) -> None:
if self.coords is None:
message = (
"Zone.coords is None. Most likely attempting to adjust vertices on "
"full-frame Zone."
)
raise RuntimeError(message)
if len(self.coords) == 0:
raise RuntimeError("Zone.coords is empty. No vertices to adjust.")

self.coords[-1] = vertex

def to_api_zone(self, stream_id: int) -> bf_codecs.Zone:
if self.name is None:
raise RuntimeError("Zone name is None. Cannot create API Zone codec.")

# BrainFrame uses a list of lists
coords = [] if self.coords is None else list(map(list, self.coords))

return bf_codecs.Zone(
name=self.name,
stream_id=stream_id,
coords=coords,
alarms=self.alarms,
id=self.id
)

@classmethod
def from_api_zone(cls, zone: bf_codecs.Zone) -> "Zone":
if zone.name == bf_codecs.Zone.FULL_FRAME_ZONE_NAME or len(zone.coords) > 2:
return Region.from_api_region(zone)
else:
return Line.from_api_line(zone)


class Line(Zone):
def copy(self) -> "Line":
return Line(
coords=self.coords.copy(),
name=self.name,
id=self.id,
alarms=self.alarms.copy()
)

def is_shape_ready(self) -> bool:
return len(self.coords) == 2

def takes_additional_points(self) -> bool:
return len(self.coords) < 2

def would_be_valid_zone(self, new_vertex: Tuple[int, int]) -> bool:
return True

@classmethod
def from_api_line(cls, zone: bf_codecs.Zone) -> "Line":
# BrainFrame uses a list of lists
coords = list(map(tuple, zone.coords))

return cls(
coords=coords,
name=zone.name,
id=zone.id,
alarms=zone.alarms,
)


class Region(Zone):
def copy(self) -> "Region":
coords = None if self.coords is None else self.coords.copy()

return Region(
coords=coords,
name=self.name,
id=self.id,
alarms=self.alarms.copy()
)

def is_shape_ready(self) -> bool:
return len(self.coords) >= 3

def takes_additional_points(self) -> bool:
return True

def would_be_valid_zone(self, new_vertex: Tuple[int, int]) -> bool:
new_coords = self.coords + [new_vertex]
shapely_polygon = geometry.Polygon(new_coords)
return shapely_polygon.is_valid

@classmethod
def from_api_region(cls, zone: bf_codecs.Zone) -> "Region":
# BrainFrame uses a list of lists
coords = None if zone.coords is None else list(map(tuple, zone.coords))

return cls(
coords=coords,
name=zone.name,
id=zone.id,
alarms=zone.alarms,
)
Loading