Add configurable resolution variables for JPEG and GIF generation#408
Open
drbergman wants to merge 2 commits intoMathCancer:developmentfrom
Open
Add configurable resolution variables for JPEG and GIF generation#408drbergman wants to merge 2 commits intoMathCancer:developmentfrom
drbergman wants to merge 2 commits intoMathCancer:developmentfrom
Conversation
GIF and JPEG resolution currently depends on simulation domain size, and `make gif` uses deprecated `magick convert` syntax.
- **Added configurable variables** for ImageMagick density and resize parameters:
```makefile
MAGICK_DENSITY := 96 # DPI for rasterization
MAGICK_RESIZE_X := 1024 # Target width
MAGICK_RESIZE_Y := 1024 # Target height
MAGICK_RESIZE := $(MAGICK_RESIZE_X)x$(MAGICK_RESIZE_Y)
```
- **Simplified `jpeg` target** from 8 lines with temporary files to single command:
```makefile
jpeg:
magick mogrify -density $(MAGICK_DENSITY) -format jpg -resize $(MAGICK_RESIZE) $(OUTPUT)/s*.svg
```
- **Updated `gif` target** to use modern syntax and add resolution control:
```makefile
gif:
magick -density $(MAGICK_DENSITY) $(OUTPUT)/s*.svg -resize $(MAGICK_RESIZE) $(OUTPUT)/out.gif
```
Override from command line for custom resolution:
```bash
make jpeg MAGICK_DENSITY=300 MAGICK_RESIZE_X=2048 MAGICK_RESIZE_Y=2048
```
Updated 20 Makefiles: main repository, Makefile-default, all 17 sample projects, and template_BM intracellular project.
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>robust resolution in jpeg and gif</issue_title>
> <issue_description>As hinted at in a recent Slack post[^1], GIF resolution depends on the domain size. The same is true for `make jpeg`.
>
> Something like
>
> ```magick -density 300 $(OUTPUT)/s*.svg -resize 1024x1024 $(OUTPUT)/out.gif```
>
> works to resolve this (and a similar command for the `jpeg` target). The issue with this simple fix is that for larger domains, this would rasterize at much higher resolutions than necessary (don't always need `-density 300`) and then would resize out of resolution (`-resize 1024x1024`).
>
> So, something more robust would be needed. I see two options:
> - do the math in the Makefile
> - doing the math when writing the SVG[^2]
>
> Either way, exposing these through Makefile variables would be a nice touch for users.
>
> ## Proposed solution
> Simplest is to introduce new Makefile variables that set defaults that can be customized:
> ```make
> MAGICK_DENSITY := 96
> MAGICK_RESIZE_X := 1024
> MAGICK_RESIZE_Y := 1024
> MAGICK_RESIZE := $(MAGICK_RESIZE_X)x$(MAGICK_RESIZE_Y)
>
> jpeg:
> magick mogrify -density $(MAGICK_DENSITY) -format jpg -resize $(MAGICK_RESIZE) $(OUTPUT)/s*.svg
>
> gif:
> magick -density $(MAGICK_DENSITY) $(OUTPUT)/s*.svg -resize $(MAGICK_RESIZE) $(OUTPUT)/out.gif
> ```
> Notes:
> 1. This simplifies the `jpeg` target down to one line, eliminating need for intermediate files to store height and width
> 2. I am now getting a deprecation warning for `magick convert` (currently used in the `gif` target), so we'll eventually need to migrate to this new syntax, but I don't know if that will disrupt others with an older installation of magick...
>
> ## For future reference
> Here's my understanding of how all the numbers are computed after looking into this:
> 1. PhysiCell records in the `<svg>` element the `width` and `height` attributes. These are the simulation domain size plus necessary padding for title text and colorbar (if needed). Title text padding scales with the domain size so if you display the snapshot as a 1-inch-wide figure, the text is always the same size. The colorbar has a fixed padding (which is what causes it to render different widths and sometimes pushes the label outside the clipping mask.
> 2. Magick interprets these as the number of pixels in each dimension and computes a desired physical size of the image in inches using 1"=96pixels. E.g. 960 pixels --> 10 inches
> 3. The `-density` flag converts this desired size (in inches) to pixels, treating the flag value as dpi. E.g., `-density 300` would take the 10" above and set that dimension to rasterize to `300 x 10 = 3000` pixels.
> 4. Rasterize at these dimensions
> 5. If `-resize` is provided, then downsample (if either dimension has more pixels from above then requested in this flag), preserving the aspect ratio. Minor note: if the `-resize` value ends with `!` (e.g. `-resize 1000x1000!`), this will force *both* dimensions; i.e. not preserve the aspect ratio. To finish our example, if `-resize 1000x1000` and the rasterized intermediate was `3000x6000` pixels, the final image will be `500x1000` pixels
>
> [^1]: When we reduce the domain size via the config file, we're noticed that the gifs generated with magick appear to have lower resolution. Is there a way to maintain or improve the resolution when using a smaller domain? The output SVGs seem to retain their quality.
> [^2]: `width` and `height` attributes control the resolution. Those attributes are dependent on the simulation domain size, but are not equivalent due to padding for text / colorbar, so those could be good targets to put default sizes.</issue_description>
>
</details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GIF and JPEG resolution currently depends on simulation domain size, and
make gifuses deprecatedmagick convertsyntax.Changes
Added configurable variables for ImageMagick density and resize parameters:
Simplified
jpegtarget from 8 lines with temporary files to single command:jpeg: magick mogrify -density $(MAGICK_DENSITY) -format jpg -resize $(MAGICK_RESIZE) $(OUTPUT)/s*.svgUpdated
giftarget to use modern syntax and add resolution control:gif: magick -density $(MAGICK_DENSITY) $(OUTPUT)/s*.svg -resize $(MAGICK_RESIZE) $(OUTPUT)/out.gifUsage
Override from command line for custom resolution:
Scope
Updated 20 Makefiles: main repository, Makefile-default, all 17 sample projects, and template_BM intracellular project.