-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat(ui): expanded thumbnail and preview features #390
Conversation
- Fix RAW images not being loaded correctly in the preview panel - Fix trying to read size data from null images - Refactor `os.stat` to `<Path object>.stat()` - Remove unnecessary upper/lower conversions - Improve encoding compatibility beyond UTF-8 when reading text files - Code cleanup
The ability to pass a border radius scaling argument is also included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved when comments are resolved.
pydub==0.25.1 | ||
mutagen==1.47.0 | ||
numpy==1.26.4 | ||
ffmpeg-python==0.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no update to the README to tell the user to install ffmpeg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm, video thumbnails/previews do not work without ffmpeg installed. (Fails to find ffprobe). Could this be something looked into being bundled into pyinstaller? Ffmpeg will be an incredibly useful tool for the future (could probably look into dropping opencv potentially)
@@ -13,7 +13,7 @@ class ColorType(int, Enum): | |||
DARK_ACCENT = 4 | |||
|
|||
|
|||
_TAG_COLORS = { | |||
_TAG_COLORS: dict = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other dict like mapping structures that might be more suitable. like defaultdict.
This can solve the keyerror solution from get_tag_color
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultdict
seems like it's well-suited for this, and would indeed replace the need for the try/except block in get_tag_color
. However, since #332 is doing some refactor work in palette.py
, I feel it would be best to wait until that one is merged to consider switching to defaultdict
.
Custom user-defined tag colors is also a feature I've been wanting to add in the near future (post-#332) that would see this getting shaken up once more. Still, it's good to be aware of defaultdict
now!
tagstudio/src/core/palette.py
Outdated
ColorType.DARK_ACCENT: "#888888", | ||
}, | ||
} | ||
|
||
|
||
def get_tag_color(type, color): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try not to PEP-8 people, but don't overwrite builtins, it never leads to a good place.
def get_tag_color(type, color): | |
def get_tag_color(type_, color): | |
color = color.lower() | |
try: | |
if type == ColorType.TEXT: | |
return get_tag_color(_TAG_COLORS[color][type_], color) |
tagstudio/src/core/palette.py
Outdated
def get_ui_color(type: ColorType, color: str): | ||
"""Returns a hex value given a color name and ColorType.""" | ||
color = color.lower() | ||
return _UI_COLORS.get(color).get(type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try not to PEP-8 people, but don't overwrite builtins, it never leads to a good place.
def get_ui_color(type: ColorType, color: str): | |
"""Returns a hex value given a color name and ColorType.""" | |
color = color.lower() | |
return _UI_COLORS.get(color).get(type) | |
def get_ui_color(type_: ColorType, color: str): | |
"""Returns a hex value given a color name and ColorType.""" | |
color = color.lower() | |
return _UI_COLORS.get(color).get(type_) |
except ffmpeg.Error: | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this an exception that is also raised if ffmpeg is not installed on the system? or otherwise not available via PATH
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great question - If so, it wasn't my intention to catch that with this method
pic = ImageChops.hard_light( | ||
pic, Image.new("RGB", size, color) | ||
) | ||
# collage.paste(pic, (y*thumb_size, x*thumb_size)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line needed?
tagstudio/src/qt/widgets/fields.py
Outdated
@@ -47,6 +48,10 @@ def __init__(self, title: str = "Field", inline: bool = True) -> None: | |||
button_size = 24 | |||
# self.setStyleSheet('border-style:solid;border-color:#1e1a33;border-radius:8px;border-width:2px;') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line needed?
""" | ||
radius_scale: float = 1 | ||
if scale_radius: | ||
radius_scale = max(size[0], size[1]) / 512 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider a name instead of a magic number.
# stroke_width=math.ceil(size / 96), | ||
# stroke_fill="#FFFF00", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a note for this one in the surrounding code:
# NOTE: While a stroke effect is desired, the text
# method only allows for outer strokes, which looks
# a bit weird when rendering fonts.
Essentially it's code I would like to use to style the font previews in a way that matches other previews like the audio waveform, but the way the stroke_width
ends up getting applied on the inside makes the previews harder to read. I left it as a quick way to reimplement it in the case a solution to the stroke alignment was found.
if ThumbRenderer.FONT_PIXEL_RATIO != pixel_ratio: | ||
ThumbRenderer.FONT_PIXEL_RATIO = pixel_ratio | ||
ThumbRenderer.ext_font = ImageFont.truetype( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break the class if you in the future subclass it. Save this to the instance or use a dataclass to store constants or otherwise don't mix class and instance in the same method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When going in to fix this I discovered that that neither FONT_PIXEL_RATIO
nor ext_font
are actually being used... Welp, takes care of that issue...
Handle ValueErrors in `render()`. This case was encountered when attempting to render an `XPM` file during testing.
Thank you for the review! I've pushed some changes that should address most of the comments. The FFmpeg support will probably need to be further resolved/clarified, but for now I've added an additional notice on the README under the installation instructions. |
* Fix text and RAW image handling - Fix RAW images not being loaded correctly in the preview panel - Fix trying to read size data from null images - Refactor `os.stat` to `<Path object>.stat()` - Remove unnecessary upper/lower conversions - Improve encoding compatibility beyond UTF-8 when reading text files - Code cleanup * Use chardet for character encoding detection * Add support for waveform + album cover thumbnails * Rename "cover" variables for MyPy * Rename "audio_tags" variables for MyPy + typing * Add # type: ignore to fromstring method * Add GIF preview support * Add rough check for invalid video codecs * Add ".plist" to PLAINTEXT_TYPES * Add readable video tester * Add ".psd" to IMAGE_TYPES; Handle ID3NoHeaderError * Improve and style waveform previews * Add final return statement to _album_artwork() * Add final return statement to _audio_waveform() * Tweak waveform color and size * Fix ItemThumb label text color in light mode * Fix most theme UI legibility issues * Match additional UI to color scheme * ruff format * feat(ui): add UI color palette dict * feat(ui) center and color small font previews * fix(ui): large font previews follow app theme * fix(ui): blender previews follow app theme * feat(ui): add resizable thumbnail options * fix: mkv files with "[0][0][0][0]" codec load properly * fix: missing audio files properly handled * feat(ui): use system accent color for thumb selections * fix(ui): hide gif preview in multi-selections * feat(ui): add dynamic file thumb icons * fix(ui): hide previous thumbnail before resizing * (fix): catch ffmpeg errors in file tester * Squashed commit of the following: commit 9a3c19d Author: Travis Abendshien <[email protected]> Date: Wed Jul 24 22:57:32 2024 -0700 fix: add missing comma + sort extensions commit 53b2db9 Author: Travis Abendshien <[email protected]> Date: Wed Jul 24 14:46:16 2024 -0700 refactor: move type constants to new media classes * feat(ui): add media types and icon resources * feat(ui): add more default media types and icons Add additional default icons for: - Blender - Presentation - Program - Spreadsheet Add/expand additional media types: - PDF - Packages * fix: remove leading dot in preview panel ext * refactor: remove edge from `four_corner_gradient()` * fix: handle missing files in `resource_manager` * fix(ui): thumb edges fading on refresh * feat(ui): add default icons for audio+vector thumbs * feat(ui): apply edge to default icon thumbs * chore: remove unused code * refactor(ui): move loading icon to `ResourceManager` * fix(ui) color for default icons follow theme * fix: remove `theme_color` redef * refactor: make some consts and args clearer * refactor: organize arguments, update docstrings The ability to pass a border radius scaling argument is also included. * chore: format docstrings with ruff * refactor: replace magic numbers with named values * refactor: remove unused code, comments, & imports * refactor: rename args to not shadow builtins * refactor: remove unused vars from `thumb_renderer` * fix: handle ValueError in `render()` Handle ValueErrors in `render()`. This case was encountered when attempting to render an `XPM` file during testing. * docs: add FFmpeg requirement to README
Ports the following thumbnail and related PRs from the `Alpha-v9.4` branch to `main` (v9.5+): - (#273) Blender thumbnail support - (#307) Add font thumbnail preview support - (#331) refactor: move type constants to new media classes - (#390) feat(ui): expanded thumbnail and preview features - (#370) ui: "open in explorer" action follows os name - (#373) feat(ui): preview support for source engine files - (#274) Refactor video_player.py (Fix #270) - (#430) feat(ui): show file creation/modified dates + restyle path label - (#471) fix(ui): use default audio icon if ffmpeg is absent - (#472) fix(ui): use birthtime for creation time on mac & win Co-Authored-By: Ethnogeny <[email protected]> Co-Authored-By: Theasacraft <[email protected]> Co-Authored-By: SupKittyMeow <[email protected]> Co-Authored-By: EJ Stinson <[email protected]> Co-Authored-By: Sean Krueger <[email protected]>
* feat: port v9.4 thumbnail + related feats to v9.5 Ports the following thumbnail and related PRs from the `Alpha-v9.4` branch to `main` (v9.5+): - (#273) Blender thumbnail support - (#307) Add font thumbnail preview support - (#331) refactor: move type constants to new media classes - (#390) feat(ui): expanded thumbnail and preview features - (#370) ui: "open in explorer" action follows os name - (#373) feat(ui): preview support for source engine files - (#274) Refactor video_player.py (Fix #270) - (#430) feat(ui): show file creation/modified dates + restyle path label - (#471) fix(ui): use default audio icon if ffmpeg is absent - (#472) fix(ui): use birthtime for creation time on mac & win Co-Authored-By: Ethnogeny <[email protected]> Co-Authored-By: Theasacraft <[email protected]> Co-Authored-By: SupKittyMeow <[email protected]> Co-Authored-By: EJ Stinson <[email protected]> Co-Authored-By: Sean Krueger <[email protected]> * remove vscode exceptions from `.gitignore` * delete .vscode directory * style: format for `ruff check` * fix(tests): update `test_update_widgets_not_selected` test * remove Send2Trash dependency * refactor: use dataclass for MediaCateogry * refactor: use enums for UI colors * docs: add file docstring for silent_Popen * refactor: replace logger with structlog * use early return inside `ResourceManager.get()` * add `is_ext_in_category()` method to `MediaCategory` Add method to check if an extension is a member of a given MediaCategory. * style: fix docstring style, missing type hints, rename `afm` * fix: use structlog vars in logging * refactor: move platform-dependent strings to PlatformStrings * refactor: move `parents[2]` path to variable * fix: undo logger regressions --------- Co-authored-by: Ethnogeny <[email protected]> Co-authored-by: Theasacraft <[email protected]> Co-authored-by: SupKittyMeow <[email protected]> Co-authored-by: EJ Stinson <[email protected]> Co-authored-by: Sean Krueger <[email protected]>
NOTE: This PR ended up becoming larger in scope than I originally planned, and normally I would've preferred to separate out some of the specific thumbnail and preview panel additions, however since
thumb_renderer.py
gets a heavy refactor in this it all ended up piling together in one branch. That being said the changes are at least all relevant to each other, even if the scope is a bit bloated.This PR introduces several new changes and features for thumbnails and previews:
Thumbnails
Preview Panel
Other UI
Closes #31, Closes #338