D12388: Output device color curves correction

[Roman Gilg]

romangg added inline comments.

INLINE COMMENTS

> zzag wrote in outputconfiguration.cpp:184
> Maybe `static_cast`?
> 
> I haven't used wl_array but can't you allocate big enough contiguous chunk of memory and call memcpy, e.g.
> 
>   wl_array wlRed;
>   
>   wl_array_init(&wlRed);
>   auto* redDest = wl_array_add(&wlRed, sizeof(uint16_t) * red.count());
>   memcpy(redDest, red.data(), sizeof(uint16_t) * red.count());

Good point about allocating directly all of the memory. I looked up the wl_array definition and should be no problem.

> cfeck wrote in outputconfiguration.h:211
> The documentation could state how many elements need to be in the vectors. Ideally, every component could have any number of elements, and if there are too few, the other elements are interpolated.
> 
> But it is probably simpler to just state "The number of elements in each vector must be 256 or 1024, depending on the depth of the framebuffer (24 bits or 30 bits)."

Number of elements depends on the OutputDevice technical possibilities (i.e. for example as you said 256 for 8bit color ramps and 1024 for 10bit ones and every other value between 1 and 16bit ramp size).

Will add a comment to the doc.

> cfeck wrote in outputconfiguration_interface.cpp:217
> X11 uses 16 bit values even for 24 bit screens, because the actual values that are sent to the DAC can have higher precision than the screen pixmap. I doubt that Wayland has reduced the precision, so I don't think checking values here is right.

I forget to remove this comment. This is not an issue since we can "stretch" the image over the whole uint16_t value domain linearly such that image 1 is associated with the largest uint16_t value. By that we have well defined arguments in any case.

The array size though is checked before that in the checkArg call by comparing it with the size set first by the compositor.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12388

To: romangg, #frameworks
Cc: zzag, cfeck, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180501/22d7a71f/attachment.html>