mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-29 02:32:01 +01:00
Fix Camera Raw preview color management (#3427)
* Fix Camera Raw preview color management * Apply canvas color management to refined previews * Camera Raw detail preview: clippy and rustfmt allow too_many_arguments on DetailPreview::paint (the display joins seven existing params) and as_chunks for the RGBA8 rows (clippy 1.99). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Brandon Thomas <echelon@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Brandon Thomas
Claude Opus 5.5
parent
88ffd89e9f
commit
a97cfd103e
@@ -29,7 +29,7 @@ pub(crate) struct DetailPreview {
|
||||
cancel: Arc<AtomicBool>,
|
||||
pub(crate) error: Option<String>,
|
||||
failed_revision: Option<u64>,
|
||||
crop: Option<(u64, bool, PixelRect)>,
|
||||
crop: Option<(u64, bool, PixelRect, u64)>,
|
||||
texture: Option<TextureHandle>,
|
||||
overlay_texture: Option<TextureHandle>,
|
||||
overlay_key: Option<OverlayKey>,
|
||||
@@ -153,7 +153,17 @@ impl DetailPreview {
|
||||
|
||||
/// Draw only the visible pixel rectangle. No full-image GPU texture and no GPU-size limit
|
||||
/// on the source image; uploads are capped to the current viewport plus a one-pixel border.
|
||||
pub(crate) fn paint(&mut self, ui: &egui::Ui, image: Rect, revision: u64, before: bool, params: &CameraRaw, needed: bool) -> bool {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn paint(
|
||||
&mut self,
|
||||
ui: &egui::Ui,
|
||||
image: Rect,
|
||||
revision: u64,
|
||||
before: bool,
|
||||
params: &CameraRaw,
|
||||
needed: bool,
|
||||
display: &photocraft_engine::display_color::CanvasDisplay,
|
||||
) -> bool {
|
||||
self.ready = false;
|
||||
self.poll(revision);
|
||||
if !needed {
|
||||
@@ -174,9 +184,9 @@ impl DetailPreview {
|
||||
} else {
|
||||
return false;
|
||||
};
|
||||
let key = (if original { 0 } else { revision }, original, crop);
|
||||
let key = (if original { 0 } else { revision }, original, crop, display.key);
|
||||
if self.crop != Some(key) {
|
||||
let color = crop_image(surface, crop);
|
||||
let color = crop_image(surface, crop, display);
|
||||
let options = TextureOptions { magnification: egui::TextureFilter::Nearest, ..TextureOptions::LINEAR };
|
||||
match &mut self.texture {
|
||||
Some(t) => t.set(color, options),
|
||||
@@ -218,7 +228,7 @@ impl DetailPreview {
|
||||
if !self.ready {
|
||||
return false;
|
||||
}
|
||||
let Some((r, original, crop)) = self.crop else { return false };
|
||||
let Some((r, original, crop, _display_key)) = self.crop else { return false };
|
||||
let t = crate::theme::Tokens::get(ui.ctx());
|
||||
let key = (r, original, crop, mode, t.kind);
|
||||
if self.overlay_key != Some(key) {
|
||||
@@ -331,14 +341,15 @@ fn visible_pixels(image: Rect, viewport: Rect, area: PixelRect, max_side: usize)
|
||||
(crop.width() as usize <= max_side && crop.height() as usize <= max_side && crop.width() as u64 * crop.height() as u64 <= 16_777_216).then_some(crop)
|
||||
}
|
||||
|
||||
fn crop_image(surface: &Surface, area: PixelRect) -> ColorImage {
|
||||
fn crop_image(surface: &Surface, area: PixelRect, display: &photocraft_engine::display_color::CanvasDisplay) -> ColorImage {
|
||||
let width = area.width() as usize;
|
||||
let mut row = vec![[0.0; 4]; width];
|
||||
let mut pixels = Vec::with_capacity(width * area.height() as usize);
|
||||
let enc = |v: f32| (v.clamp(0.0, 1.0) * 255.0 + 0.5) as u8;
|
||||
for y in area.y0..area.y1 {
|
||||
surface.read_rgba_into(PixelRect::new(area.x0, y, area.x1, y + 1), &mut row);
|
||||
pixels.extend(row.iter().map(|q| Color32::from_rgba_unmultiplied(enc(q[0]), enc(q[1]), enc(q[2]), enc(q[3]))));
|
||||
let buf = photocraft_compose::Buffer { rect: photocraft_geom::Rect::new(area.x0, y, area.x1, y + 1), px: row.clone() };
|
||||
let img = display.to_rgba8(&buf);
|
||||
pixels.extend(img.pixels.as_chunks::<4>().0.iter().map(|&[r, g, b, a]| Color32::from_rgba_unmultiplied(r, g, b, a)));
|
||||
}
|
||||
ColorImage::new([width, area.height() as usize], pixels)
|
||||
}
|
||||
@@ -410,13 +421,32 @@ mod tests {
|
||||
let dark = preview.sample([0.0, 0.0], 1, false, ¶ms).unwrap();
|
||||
let light = preview.sample([1.0 / 40.0, 0.0], 1, false, ¶ms).unwrap();
|
||||
assert!(light[0] - dark[0] > 0.5, "native alternating pixels survive: {mode:?}/{depth:?}");
|
||||
assert_eq!(crop_image(&expected, PixelRect::new(-12, 8, -10, 9)).size, [2, 1]);
|
||||
let mut doc =
|
||||
photocraft_doc::Document::new("test", photocraft_geom::Size::new(40, 30), photocraft_doc::ColorMode::Rgb, photocraft_doc::SampleType::U8);
|
||||
doc.icc_profile = Some(photocraft_cms::Builtin::Srgb.profile().to_bytes());
|
||||
let display = photocraft_engine::color_cmds::ColorState::default().canvas_display(&doc).unwrap();
|
||||
assert_eq!(crop_image(&expected, PixelRect::new(-12, 8, -10, 9), &display).size, [2, 1]);
|
||||
assert!(preview.sample([f32::NAN, 0.0], 1, false, ¶ms).is_none());
|
||||
assert!(preview.sample([0.5, 0.5], 2, false, ¶ms).is_none());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refined_crop_uses_linear_document_display_encoding() {
|
||||
let area = PixelRect::new(0, 0, 1, 1);
|
||||
let mut source =
|
||||
Surface::new(photocraft_color::PixelFormat { mode: photocraft_color::ColorMode::Rgb, sample: photocraft_color::SampleType::F32, alpha: true });
|
||||
source.write_region(area, &[0.25, 0.25, 0.25, 1.0]);
|
||||
let mut doc = photocraft_doc::Document::new("test", photocraft_geom::Size::new(1, 1), photocraft_doc::ColorMode::Rgb, photocraft_doc::SampleType::F32);
|
||||
doc.icc_profile = Some(photocraft_cms::Builtin::LinearSrgb.profile().to_bytes());
|
||||
let mut color = photocraft_engine::color_cmds::ColorState::default();
|
||||
color.settings.monitor_profile = "srgb".into();
|
||||
let display = color.canvas_display(&doc).unwrap();
|
||||
let image = crop_image(&source, area, &display);
|
||||
assert!(image.pixels[0].r() > 130, "linear 0.25 should be shown as encoded sRGB");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn huge_sparse_images_keep_the_proxy_without_starting_an_unbounded_worker() {
|
||||
let source = Surface::new(PixelFormat::RGBA8);
|
||||
|
||||
@@ -44,10 +44,12 @@ pub struct CameraRawDialog {
|
||||
full_h: usize,
|
||||
preview_ppp: f32,
|
||||
pub(crate) detail: super::camera_raw_detail_ui::DetailPreview,
|
||||
display: std::sync::Arc<photocraft_engine::display_color::CanvasDisplay>,
|
||||
pub(crate) viewport: Option<ERect>,
|
||||
pub(crate) float: bool,
|
||||
tex: Option<TextureHandle>,
|
||||
before_tex: Option<TextureHandle>,
|
||||
before_display_key: Option<u64>,
|
||||
pub(crate) before_histogram: RgbHistogram,
|
||||
pub(crate) processed: Vec<[f32; 4]>,
|
||||
pub(crate) lab_transform: std::sync::Arc<photocraft_cms::Transform>,
|
||||
@@ -116,9 +118,10 @@ impl CameraRawDialog {
|
||||
non_defaults(&self.params)
|
||||
}
|
||||
|
||||
fn image(px: &[[f32; 4]], w: usize, h: usize) -> egui::ColorImage {
|
||||
let enc = |v: f32| (v.clamp(0.0, 1.0) * 255.0 + 0.5) as u8;
|
||||
egui::ColorImage::new([w, h], px.iter().map(|q| Color32::from_rgba_unmultiplied(enc(q[0]), enc(q[1]), enc(q[2]), enc(q[3]))).collect())
|
||||
fn image(px: &[[f32; 4]], w: usize, h: usize, display: &photocraft_engine::display_color::CanvasDisplay) -> egui::ColorImage {
|
||||
let buf = photocraft_compose::Buffer { rect: Rect::new(0, 0, w as i32, h as i32), px: px.to_vec() };
|
||||
let img = display.to_rgba8(&buf);
|
||||
egui::ColorImage::from_rgba_unmultiplied([img.width as usize, img.height as usize], &img.pixels)
|
||||
}
|
||||
|
||||
pub(crate) fn render(&mut self, ctx: &egui::Context) {
|
||||
@@ -137,13 +140,18 @@ impl CameraRawDialog {
|
||||
let histogram_start = crate::gpu_canvas::now_ms();
|
||||
self.after_histogram = RgbHistogram::from_rgba(&px);
|
||||
self.histogram_ms = crate::gpu_canvas::now_ms() - histogram_start;
|
||||
let img = Self::image(&px, self.pw, self.ph);
|
||||
let img = Self::image(&px, self.pw, self.ph, &self.display);
|
||||
match &mut self.tex {
|
||||
Some(t) => t.set(img, egui::TextureOptions::LINEAR),
|
||||
None => self.tex = Some(ctx.load_texture("camera-raw-after", img, egui::TextureOptions::LINEAR)),
|
||||
}
|
||||
if self.before_tex.is_none() {
|
||||
self.before_tex = Some(ctx.load_texture("camera-raw-before", Self::image(&self.proxy, self.pw, self.ph), egui::TextureOptions::LINEAR));
|
||||
if self.before_display_key != Some(self.display.key) {
|
||||
let before = Self::image(&self.proxy, self.pw, self.ph, &self.display);
|
||||
match &mut self.before_tex {
|
||||
Some(t) => t.set(before, egui::TextureOptions::LINEAR),
|
||||
None => self.before_tex = Some(ctx.load_texture("camera-raw-before", before, egui::TextureOptions::LINEAR)),
|
||||
}
|
||||
self.before_display_key = Some(self.display.key);
|
||||
}
|
||||
self.processed = px;
|
||||
self.scope.cache = None;
|
||||
@@ -521,6 +529,7 @@ fn open_pixels(
|
||||
Coverage::None => None,
|
||||
};
|
||||
let source = photocraft_engine::color_cmds::composite_profile(&st.doc);
|
||||
let display = app.session.color.canvas_display(&st.doc).map_err(|e| e.to_string())?;
|
||||
let lab_transform = photocraft_cms::cached(&source, photocraft_cms::Builtin::LabD50.profile(), Default::default()).map_err(|e| e.to_string())?;
|
||||
let scope_transform = photocraft_cms::cached(&source, photocraft_cms::Builtin::Srgb.profile(), Default::default()).map_err(|e| e.to_string())?;
|
||||
if let Some(saved) = app.session.prefs().dialogs.get("filter.cameraRaw.scope")
|
||||
@@ -569,10 +578,12 @@ fn open_pixels(
|
||||
Coverage::None => super::camera_raw_detail_ui::Coverage::None,
|
||||
},
|
||||
),
|
||||
display,
|
||||
viewport: None,
|
||||
float,
|
||||
tex: None,
|
||||
before_tex: None,
|
||||
before_display_key: None,
|
||||
dirty: true,
|
||||
show_before: false,
|
||||
mixer_tab: 1,
|
||||
@@ -879,9 +890,16 @@ impl CameraRawDialog {
|
||||
fn draw(app: &mut PhotocraftApp, ctx: &egui::Context, own_window: bool) {
|
||||
let t = Tokens::get(ctx);
|
||||
let screen = ctx.content_rect();
|
||||
let current_display = app.session.active().and_then(|st| app.session.color.canvas_display(&st.doc).ok());
|
||||
let mut action: Option<&str> = None;
|
||||
egui::Area::new(egui::Id::new("camera-raw-dialog")).order(egui::Order::Foreground).fixed_pos(screen.min).show(ctx, |ui| {
|
||||
let Some(d) = app.camera_raw.as_mut() else { return };
|
||||
if let Some(display) = ¤t_display
|
||||
&& display.key != d.display.key
|
||||
{
|
||||
d.display = display.clone();
|
||||
d.dirty = true;
|
||||
}
|
||||
d.curve_rect = None;
|
||||
d.scope.vectorscope_rect = None;
|
||||
if d.dirty {
|
||||
@@ -920,7 +938,7 @@ fn draw(app: &mut PhotocraftApp, ctx: &egui::Context, own_window: bool) {
|
||||
widgets::checker(preview.painter(), r.intersect(view), 8.0);
|
||||
preview.painter().image(tex.id(), r, ERect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE);
|
||||
let detail_needed = r.width() * ctx.pixels_per_point() > d.pw as f32 || r.height() * ctx.pixels_per_point() > d.ph as f32;
|
||||
d.detail.paint(&preview, r, d.preview_revision, d.show_before, &d.params, detail_needed);
|
||||
d.detail.paint(&preview, r, d.preview_revision, d.show_before, &d.params, detail_needed, &d.display);
|
||||
if d.detail.pending() {
|
||||
ctx.request_repaint_after(std::time::Duration::from_millis(50));
|
||||
}
|
||||
@@ -1113,6 +1131,20 @@ fn hue_color(h: f32) -> Color32 {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn preview_texture_uses_linear_document_display_encoding() {
|
||||
let mut doc = photocraft_doc::Document::new("test", photocraft_geom::Size::new(1, 1), photocraft_doc::ColorMode::Rgb, photocraft_doc::SampleType::F32);
|
||||
doc.icc_profile = Some(photocraft_cms::Builtin::LinearSrgb.profile().to_bytes());
|
||||
let mut color = photocraft_engine::color_cmds::ColorState::default();
|
||||
color.settings.monitor_profile = "srgb".into();
|
||||
let display = color.canvas_display(&doc).unwrap();
|
||||
let px = [[0.25, 0.25, 0.25, 1.0]];
|
||||
let preview = CameraRawDialog::image(&px, 1, 1, &display);
|
||||
let expected = display.to_rgba8(&photocraft_compose::Buffer::filled(Rect::new(0, 0, 1, 1), px[0]));
|
||||
assert_eq!(preview.pixels[0].to_array(), expected.pixels[..4]);
|
||||
assert_ne!(preview.pixels[0].r(), 64, "linear light must be encoded before display");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn histogram_follows_preview_and_view_changes_do_not_render_or_edit() {
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
|
||||
|
||||
Reference in New Issue
Block a user