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

Improvements to Image and Grid Image Rendering #13

Merged
merged 37 commits into from
May 3, 2024
Merged

Conversation

AnonymouX47
Copy link
Owner

@AnonymouX47 AnonymouX47 commented Apr 27, 2024

  • Implements thumbnail generation (with deduplication) and caching for the image grid.
    • Adds thumbnail, thumbnail cache and thumbnail size config options.
    • Adds --thumbnail/--no-thumbnail command-line option.
  • Revamps the max pixels setting.
    • Makes the max pixels setting opt-in i.e by default, all images are now rendered regardless of resolution.
    • Changes the default value of the max pixels config option to 0 (disabled).
    • Removes --max-pixels-cli command-line option.
    • The max pixels setting no longer applies in the full-grid-image context.
    • In the image-grid context, images with more pixels than max pixels (if non-zero) are now distinguished by a yellow title and border.
  • Adds a Force Render action to the menu context.
  • Refactors and improves the performance of grid image rendering in various ways.
  • Improves error handling in image and grid image rendering.
  • Updates and improves a lot of comments.

Addresses #12 (aside the minimum grid cell width suggestion).

- Fix: Also handle possible errors during the creation of image
  instances and setting image size.
- Change: Split rendering of grid and non-grid images into separate
  functions, for simplification.

  - Add `render_grid_images()` for grid images.
  - Refactor `render_images()` for non-grid images only.

- Change: Rename input `image` -> `source`.
- Change: For grid image rendering, output `source` instead of
  `image._source`.
- Change: Purge the input queue to the renderer(s) first.
- Change: Rename variables:

  - `image_path` -> `source`
  - `image` -> `render`
  - `dir` -> `source_dirname`
  - `entry` -> `source_basename`

- Change: Update comments.
- Add: `thumbnail cache` and `thumbnail size` config options.
- Add: `--thumbnail-cache` and `--thumbnail-size` CLI options.
- Remove: `--thumbnail-cache` and `--thumbnail-size` CLI options.

Theese settings aren't the kind a user would typically want to change
per session.
- Change: Increase the upper bound from 256 to 512.
- Change: Increase the default value from 128 to 256.
- Change: Replace `.tui.main.grid_change` with
  `.tui.render.grid_renderer_in_sync`.
- Change: Reverse grid sync logic to allow blocking via `Event.wait()`
  instead of using tight loops.
- Change: Rename `new_grid` -> `delimeted` in `manage_grid_renders()`.
- Change: Update comments.
- Add: Integrate thumbnailing into the image grid rendering process.
- Add: Functions in `.tui.render`:

  - `manage_grid_thumbnails()`
  - `generate_grid_thumbnails()`
  - `delete_thumbnail()`

  for thumbnailing in the image grid.

- Add: `mark_thumbnail_rendered()` in `manage_grid_renders()`.
- Add: `.tui.main.THUMBNAIL_SIZE_PRODUCT`.
- Add: `.tui.main.THUMBNAIL_CACHE_SIZE`.
- Add: `GridThumbnailManager` thread and `GridThumbnailer`
  thread/subprocess.
- Add: A `thumbnail` field to the data passed through every grid render
  queue.
- Add: Synchronization primitives for thumbnailing, in `.tui.render`.
- Add: `Image._ti_grid_thumbnailing_threshold`, a dynamic threshold
  above which images are thumbnailed i.e any image with more pixels
  than the threshold is rendered with a thumbnail.

  This threshold is updated upon terminal resize (for graphics-based
  styles and only if terminal cell size changes) and grid cell resize.

- Add: `Image._ti_update_grid_thumbnailing_threshold()`.
- Add: `.tui.keys._prev_cell_size`.
- Change: Improve the grid thumbnailing criterion.

  - Now based on both the thumbnail size and grid cell size, not just
    the thumbnail size.
  - The new citerion prevents thumbnailing of images with more pixels
    than the thumbnail size but less than the grid cell size. It also
    ensures any image rendered with a thumbnail is larger than the
    grid cell size along at least one axis.

- Change: Render thumbnails in the grid with `FIT` sizing instead of
  `AUTO` since any image now rendered with a thumbnail is known to be
  larger than the grid cell size along at least one axis, based on the
  new thumbnailing criterion.
- Add: `.render.refresh_grid_rendering()`.
- Change: Call `refresh_grid_rendering()` in `.main` and `.keys`
  instead of repeating the same code all over.
- Change: Correct typo in comments: `delimeter` -> `delimiter`.
- Change: Refresh grid rendering regardless of whether the grid is
  active or not.
- Change: For graphics-based styles, refresh grid rendering based on
  change in terminal cell size.

  Cell ratio is now only for text-based styles.
- Change: Convert non-RGB[A] source images to RGB[A] before resizng.
- Fix: Finalize PIL images for thumbnails when thumbnail file creation
  fails.
- Add: `thumbnail` config option.
- Add: `--thumbnail` and `--no-thumbnail` CLI options.
- Add: `.main.THUMBNAIL`.
- Change: Guard thumbnailing-related operations with `.main.THUMBNAIL`.
- Change: Apply "max pixels" to grid rendering when thumbnailing is
  disabled.
- Add: `--no-max-pixels-grid` command-line option.
_ Add: `.main.NO_MAX_PIXELS_GRID`.
- Change: Do not apply "max pixels" to grid rendering when thumbnailing
  is disabled and `--no-max-pixels-grid` is specified.
- Add: Mention `--no-max-pixels-grid` where neccessary.
- Change: Update the descriptions of `max pixels` and `thumbnail`
  config options.
- Change: Update the description of `--max-pixels` command-line opton.
@AnonymouX47 AnonymouX47 added cli Related to the CLI mode config Related to viewer Configuration improvement Implementation of improvement to an existing feature new New feature implementation tui Related to the TUI (and TUI mode) labels Apr 27, 2024
@AnonymouX47 AnonymouX47 added this to the 0.2.0 milestone Apr 27, 2024
- Add: `thumbnail`, `thumbnail cache` and `thumbnail size` to the
  sample default config file.
- Change: Update the descriptions of `max pixels` config option and
  `--max-pixels` CL option.
- Change: Remove references to `--no-max-pixels-grid`.
- Change: Include zero in the value range for `max pixels` config
  option and `--max-pixels` CL option.

  A value of zero implies the absence of a maximum pixel-count i.e
  all images are rendered regardless of their resolution.

- Change: Change the default value of `max pixels` config option to
  zero.
- Change: Update the documentation of the `max pixels` config option
  and `--max-pixels` CL option.

  - Update the description, value range and default value.
  - Remove the admonition about performance implications.

- Change: Remove the footnote for the `--max-pixels` CL option; the
  repetition is unnecessary.

Turns out displaying all images regardless of resolution results in a
better out-of-the-box user experience as that's the expectation of
literally every user (other than myself 🤦).
Also, the so-called "performance implications" of displaying high-res
images turns to be really not as bad as I had it pictured in my mind.

Thanks to [@qrockz](https://github.com/qrockz) and
[@DreamMaoMao](https://github.com/DreamMaoMao) for helping me make the
right decision 😃.

Refs: #12.
- Change: Simply comparisons involving `.render.THUMBNAIL_CACHE_SIZE`
  using chained comparisons.
- Change: Correct terminology: image "size" -> image "resolution".
The option is no longer neccessary since the *max pixels* setting is
now opt-in.
- Change: Always render images in the "full-grid-image" context,
  regardless of resolution.
- Change: Remove "full-grid-image::Force Render" action.
- Change: Update the description of the `max pixels` config option.

I fail to see a need for the *max pixels* setting in this context
since full-sized views of images can/will not typically be displayed
in rapid succession.

Refs: #12 (comment)
- Add: A "high-res" pallete entry to the TUI.
- Add: A *title_attr* parameter to the `.widgets.LineSquare`
  constructor.

  Makes it possible to set any display attribute for the square's
  title.

- Change: Change the title and border color for grid cells
  containing images having pixel counts above *max pixels* to yellow,
  in order to highlight them to the user.
- Change: Update the description of the `max pixels` config option.

Refs: #12 (comment)
- Change: Add "menu::Force Render" action.
- Change: Move the "Force Render" action in the "menu", "image" and
  "full-image" contexts to after all other non-naviagtion actions
  since its priority has decreased now that *max pixels* is opt-in.
- Add: Thumbnail deduplication to grid image rendering.

  Source images with **exactly** identical thumbnails share the same
  thumbnail file.

- Add: `thumbnail_sources` to keep track of sources sharing a thumbnail
  file.
- Add: A *deduplicated* field to the output thumbnail queue items.
- Add: Deduplication feature to the `thumbnail` config option docs.
- Change: Name thumbnail files based on a hash of their content instead
  of the source file name, in order to implement/aid deduplication.
- Change: No longer guard thumbnail cleanup with `THUMNAIL_CACHE_SIZE`
  as deduplication also necessitates it.
- Change: Refactor `generate_grid_thumbnails()`.
- Change: Refactor `manage_grid_thumbnails()`.
- Change: Re-implement `mark_thumbnail_rendered()` in
  `manage_grid_renders()`.
- Change: Re-implement thumbnail eviction.

  - Now always evicts the oldest thumbnail with the least amount of
    linked sources.
  - The process is now simpler and more straightforward.
- Fix: Use `len(thumbnail_sources)` instead of `len(thumbnail_cache)`
  to get the number of thumbnails because there can be more sources
  than thumbnails as thumbnails are now deduplicated and shared among
  sources.

  The number of active thumbnail files no longer exceeds the cache size.
- Add: *alpha* parameter to `render_grid_images()`.
- Change: `GridRenderManager` no longer accepts the image canvas size
  and the alpha spec via `grid_render_queue`.

  - The alpha spec is constant.
  - `GridRenderManager` is capable of deriving the image canvas size
    on its own. In fact, already did derive the grid cell width
    which is all that's required to compute the image canvas size.

- Change: `GridRenderer`s no longer accept the alpha spec via it's input
  queue. Instead, they now accepts it as an arg since it's constant.
- Change: Rename some related/affected variables in
  `manage_grid_renders()`, `manage_grid_thumbnails()` and
  `render_grid_images()`, and remove some others.
- Change: Use `cell_width // 2` instead of `ceil(cell_width / 2)` as
  cell height since `cell_width` is known to always be even.
- Change: Rename `refresh_grid_rendering()` ->
  `resync_grid_rendering()`.
- Fix: Do not resync grid image rendering upon terminal resize if the
  grid is not active.

  Fixes a deadlock when the terminal is resized and cell size changes
  while the grid is not active.

Broken in 6ef2a51.
- Change: Update the value of `max pixels`.
@AnonymouX47 AnonymouX47 changed the title Improvements to Image and Image Grid Rendering Improvements to Image and Grid Image Rendering May 3, 2024
@AnonymouX47 AnonymouX47 merged commit b3650b2 into main May 3, 2024
16 checks passed
@AnonymouX47 AnonymouX47 deleted the rendering branch May 3, 2024 00:32
@AnonymouX47 AnonymouX47 added the args Related to the CLI (arguments and options) label May 3, 2024
AnonymouX47 added a commit that referenced this pull request May 7, 2024
- Fix: Cleanup of thumbnails upon interruption (by the user or an
  exception).
- Add: `.__main__.TEMP_DIR`.
- Change: Create a session-specific temporary data directory and clean
  it up at the topmost execution level.
- Change: Disable thumbnailing if unable to create the temporary data
  directory.
- Change: Refactor `generate_grid_thumbnails()` and
  `manage_grid_thumbnails()` in `.tui.render` accordingly.

Complements #13, #16.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
args Related to the CLI (arguments and options) cli Related to the CLI mode config Related to viewer Configuration improvement Implementation of improvement to an existing feature new New feature implementation tui Related to the TUI (and TUI mode)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant