okay, did a quick pass over the code: - not sure the lightroom import thing makes sense, it should probably be left alone or use the version 6 directly (not 5 and then add more params than exist in this version). also the code looks incomplete to me as it is.
- one stray printf left in the code the rest looks pretty straight forward and like small changes to me. opencl will transparently just work as only the data written in commit_params() is affected. i'll also need to test it. cheers, jo On Thu, Oct 8, 2015 at 4:05 AM, johannes hanika <hana...@gmail.com> wrote: > hi, > > (cross posting to new dev mailing list, re-attaching patch) > > now that we're approaching feature freeze for 2.0. do we want that > patch in 2.0 or not? i really liked the idea, but didn't check the > code yet. waiting for other things to come up in the future sounds > like a long way to go.. > > cheers, > jo > > > On Fri, Aug 14, 2015 at 3:44 AM, Nicolas Lannuzel <nlannu...@gmail.com> wrote: >> Hi Roman, >> >> Perspective correction always requires cropping, so from a GUI standpoint, I >> can understand why the keystone feature was added to the C&R module. But >> true, each interpolation step add artifacts. >> >> I can check the Python script, it doesn't seem too difficult to rewrite it >> in C. >> >> Cheers, >> Nicolas >> >> On Thu, Aug 13, 2015 at 5:28 AM, Roman Lebedev <lebedev...@gmail.com> wrote: >>> >>> On Wed, Aug 12, 2015 at 7:21 PM, Nicolas Lannuzel <nlannu...@gmail.com> >>> wrote: >>> > Hi, >>> Hello. >>> >>> > This is a small patch for the "crop&rotate" module. It adds a slider to >>> > fine-tune the amount of vertical keystone correction. The slider can >>> > move >>> > from 0 (no correction, original image) to 100% (verticals are >>> > verticals). >>> > >>> > Sometime architecture photos can look artificial if the keystone >>> > correction >>> > is fully applied. Setting the factor to 80 or 90% gives a more natural >>> > look. >>> Indeed, makes sense.. >>> >>> > I've done it by translating C and D points linearly on the X axis (in >>> > clipping.c, function commit_params): >>> > float kf = (float)(p->vert_keystone_amount / 100.0); >>> > >>> > d->kxc = d->kxb + kf * (d->kxc - d->kxb); >>> > d->kxd = d->kxa + kf * (d->kxd - d->kxa); >>> > >>> > >>> > I've also updated the legacy_params function, and the >>> > LRDT_CLIPPING_VERSION >>> > from 4 to 5. Did I miss anything and what do you think of this patch? >>> Some not really meaningful words: >>> Right now, there are 2 IOPs that do pixel interpolation - lens >>> correction and C&R. >>> This is slow[er than doing it in one step] and probably produces >>> slightly lower quality than 1 step would. >>> C&R only needs to do pixel interpolation for this perspective >>> correction and rotation, >>> just cropping can be done in without it. >>> >>> So there is a push to move perspective correction [and rotation?] into >>> lens correction module. >>> New implementation algorithm will be a bit more mathematically >>> sophisticated, >>> and in it's current version It already does have a similar function, see: >>> >>> http://sourceforge.net/p/lensfun/code/ci/a532325328117177d3cefdd7d9fcea7aafaeb805/tree/tools/perspective_control/perspective_control.py#l291 >>> [I have CC'd tools author, Bronger] >>> >>> But there is no ETA on when it will happen. >>> >>> Now, C&R iop is, well, a delicate piece of code :), so we are a little >>> scared of touching it... >>> NOTE: i'm not saying that this patch is bad or that we do not want it. >>> >>> So my personal suggestion would be: try that tool >>> [perspective_control.py], find a few issues there, >>> discuss it's functionality, etc. >>> And if you can, help rewriting it in C. I'm afraid i do not know >>> Python that well do be able to do it by myself. >>> Once that tool is in C, integration of this functionality will be much >>> closer. >>> >>> (probably that was not what you have wanted to hear, but that is my >>> view on the subject) >>> >>> > >>> > Cheers, >>> > Nicolas >>> Roman. >>> >>> > >>> > >>> > ------------------------------------------------------------------------------ >>> > >>> > _______________________________________________ >>> > darktable-devel mailing list >>> > darktable-de...@lists.sourceforge.net >>> > https://lists.sourceforge.net/lists/listinfo/darktable-devel >>> > >> >> >> >> ------------------------------------------------------------------------------ >> >> _______________________________________________ >> darktable-devel mailing list >> darktable-de...@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/darktable-devel >> ___________________________________________________________________________ darktable developer mailing list to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org
