Skip to content

Prepare demosaicer module to process whole image data - #22236

Draft
jenshannoschwalm wants to merge 4 commits into
darktable-org:masterfrom
jenshannoschwalm:demosaic_caching
Draft

Prepare demosaicer module to process whole image data#22236
jenshannoschwalm wants to merge 4 commits into
darktable-org:masterfrom
jenshannoschwalm:demosaic_caching

Conversation

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

Instead of using the given input roi we expand processed data to full piece->buff_in dimension.

This is a pre-requisite for efficient caching of demosaiced output rgb data before the internal resampling.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Core pixelpipe ROI behavior affects CPU and OpenCL image output and requires runtime integration validation.

Pull request overview

Prepares demosaic processing to operate on full-image input before resampling the requested output region.

Changes:

  • Expands demosaic input ROI to full resolution.
  • Uses full-buffer mask and RGB resampling paths.
  • Keeps CPU and OpenCL behavior aligned.
File summaries
File Description
src/iop/demosaic.c Requests full-image input and updates mask, CPU, and OpenCL resampling.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Approximate previews mishandle nonzero ROI origins, and full-image demosaicing introduces substantial uncached work.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread src/iop/demosaic.c Outdated
Comment thread src/iop/demosaic.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Legacy custom orders can misplace the scaling stage, and localization inputs omit the new module.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread src/common/iop_order.c Outdated
Comment thread data/kernels/basic.cl Outdated
Comment thread src/iop/demosaicscale.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The generated introspection source is missing from the translation extraction list.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread po/POTFILES.in Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Automatic-radius allocation failures are now silently persisted as a fallback radius.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/iop/demosaicing/capture.c
@jenshannoschwalm
jenshannoschwalm force-pushed the demosaic_caching branch 2 times, most recently from a114a91 to bd5a9e6 Compare September 11, 2026 17:46
@jenshannoschwalm
jenshannoschwalm requested a balanced review from Copilot September 11, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jenshannoschwalm
jenshannoschwalm requested a balanced review from Copilot September 11, 2026 18:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jenshannoschwalm
jenshannoschwalm requested a balanced review from Copilot September 12, 2026 07:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Instead of using the given input roi we expand processed data to full piece->buff_in dimension.

This is a pre-requisite for efficient caching of demosaiced output rgb data before the internal resampling.

The check for the approximating demosaicers got updated for thumbs, passed parameters had to be changed
as we now have to process full image data.
The preview pipe uses downscaled mipmap data anyway so absolutely no need for approximation, btw the test
didn't work anyway.
As the demosaicer now always process full image data we downscale to the required roi
in another module enabled for all demosaiced images.

Pro and against arguments for using this strategy:

About cost vs. efficacy:
1. As we now always demosaic full image data the pipe is principally slower if parameters
   change before/inside demosaic and we use a smaller roi than full data (like when using crop).
   This affects all pipe types so exports will be somewhat slower especially if we crop,
   either by crop module or ashift.
2. Also we have to process more when zooming or dragging the roi when zoomed in.
3. As demosaic does no internal crop&scale anymore, the memory footprint is lower thus internal
   tilesizes can be smaller with a possible perf gain.
4. BUT: in all desktop pipes we avoid processing all modules before the demosaic downscaler
   after the first pipe run as we have cached data and just have to crop&scale which is much
   faster than re-processing from pipe start.

About quality & results:
All demosaicers have inferior interpolators at the border regions, depending on the demosaicer
algorithm this affects the outermost 2-4 photosites.
With the old method we had these minor quality pixels always on the cropped roi.
As we now calculate all pixels we crop on the better data thus results will be somewhat better
at the outermost areas.
1. All code required for internal crop&scale could go as we demosaic full data.
2. Simper code & parameters for the approximating interpolators.
3. The capture radius calculation could be simplified too as for having full data.
As raw opposed works on full image data code could be simplified and thus it's slightly faster.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Thumbnail processing now creates a potentially enormous full-resolution RGBA intermediate before downscaling.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/iop/demosaicscale.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants