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
From 65d68dc3263715ec0f59a56bd77ec60343bdbb35 Mon Sep 17 00:00:00 2001
From: Nicolas Lannuzel <nlannu...@gmail.com>
Date: Wed, 12 Aug 2015 23:56:07 +0800
Subject: [PATCH] added a slider to tune the amount of vertical keystone
correction
---
src/develop/lightroom.c | 4 ++-
src/iop/clipping.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/src/develop/lightroom.c b/src/develop/lightroom.c
index c213352..31642ad 100644
--- a/src/develop/lightroom.c
+++ b/src/develop/lightroom.c
@@ -45,13 +45,15 @@
// 2. add LRDT_<iop_name>_VERSION with corresponding module version
// 3. use this version to pass in dt_add_hist()
-#define LRDT_CLIPPING_VERSION 4
+#define LRDT_CLIPPING_VERSION 5
typedef struct dt_iop_clipping_params_t
{
float angle, cx, cy, cw, ch, k_h, k_v;
+ float vert_keystone_amount;
float kxa, kya, kxb, kyb, kxc, kyc, kxd, kyd;
int k_type, k_sym;
int k_apply, crop_auto;
+ int ratio_n, ratio_d;
} dt_iop_clipping_params_t;
#define LRDT_FLIP_VERSION 1
diff --git a/src/iop/clipping.c b/src/iop/clipping.c
index cfaa7b2..0c66d25 100644
--- a/src/iop/clipping.c
+++ b/src/iop/clipping.c
@@ -43,7 +43,7 @@
#include <gdk/gdkkeysyms.h>
#include <assert.h>
-DT_MODULE_INTROSPECTION(5, dt_iop_clipping_params_t)
+DT_MODULE_INTROSPECTION(6, dt_iop_clipping_params_t)
#define CLAMPF(a, mn, mx) ((a) < (mn) ? (mn) : ((a) > (mx) ? (mx) : (a)))
@@ -79,6 +79,7 @@ typedef enum dt_iop_clipping_guide_t
typedef struct dt_iop_clipping_params_t
{
float angle, cx, cy, cw, ch, k_h, k_v;
+ float vert_keystone_amount;
float kxa, kya, kxb, kyb, kxc, kyc, kxd, kyd;
int k_type, k_sym;
int k_apply, crop_auto;
@@ -109,10 +110,10 @@ int legacy_params(dt_iop_module_t *self, const void *const old_params, const int
void *new_params, const int new_version)
{
if(new_version <= old_version) return 1;
- if(new_version != 5) return 1;
+ if(new_version != 6) return 1;
dt_iop_clipping_params_t *n = (dt_iop_clipping_params_t *)new_params;
- if(old_version == 2 && new_version == 5)
+ if(old_version == 2 && new_version == 6)
{
// old structure def
typedef struct old_params_t
@@ -157,8 +158,9 @@ int legacy_params(dt_iop_module_t *self, const void *const old_params, const int
// will be computed later, -2 here is used to detect uninitialized value, -1 is already used for no
// clipping.
n->ratio_d = n->ratio_n = -2;
+ n->vert_keystone_amount = 100.0;
}
- if(old_version == 3 && new_version == 5)
+ if(old_version == 3 && new_version == 6)
{
// old structure def
typedef struct old_params_t
@@ -185,8 +187,9 @@ int legacy_params(dt_iop_module_t *self, const void *const old_params, const int
// will be computed later, -2 here is used to detect uninitialized value, -1 is already used for no
// clipping.
n->ratio_d = n->ratio_n = -2;
+ n->vert_keystone_amount = 100.0;
}
- if(old_version == 4 && new_version == 5)
+ if(old_version == 4 && new_version == 6)
{
typedef struct old_params_t
{
@@ -210,6 +213,32 @@ int legacy_params(dt_iop_module_t *self, const void *const old_params, const int
// will be computed later, -2 here is used to detect uninitialized value, -1 is already used for no
// clipping.
n->ratio_d = n->ratio_n = -2;
+ n->vert_keystone_amount = 100.0;
+ }
+ if(old_version == 5 && new_version == 6)
+ {
+ typedef struct old_params_t
+ {
+ float angle, cx, cy, cw, ch, k_h, k_v;
+ float kxa, kya, kxb, kyb, kxc, kyc, kxd, kyd;
+ int k_type, k_sym;
+ int k_apply, crop_auto;
+ int ratio_n, ratio_d;
+ } old_params_t;
+
+ old_params_t *o = (old_params_t *)old_params;
+
+ n->angle = o->angle, n->cx = o->cx, n->cy = o->cy, n->cw = o->cw, n->ch = o->ch;
+ n->k_h = o->k_h, n->k_v = o->k_v;
+ n->kxa = o->kxa, n->kxb = o->kxb, n->kxc = o->kxc, n->kxd = o->kxd;
+ n->kya = o->kya, n->kyb = o->kyb, n->kyc = o->kyc, n->kyd = o->kyd;
+ n->k_type = o->k_type;
+ n->k_sym = o->k_sym;
+ n->k_apply = o->k_apply;
+ n->crop_auto = o->crop_auto;
+ n->ratio_n = o->ratio_n;
+ n->ratio_d = o->ratio_d;
+ n->vert_keystone_amount = 100.0;
}
return 0;
@@ -217,6 +246,7 @@ int legacy_params(dt_iop_module_t *self, const void *const old_params, const int
typedef struct dt_iop_clipping_gui_data_t
{
GtkWidget *angle;
+ GtkWidget *vert_keystone_amount;
GtkWidget *hvflip;
GList *aspect_list;
@@ -251,6 +281,7 @@ typedef struct dt_iop_clipping_gui_data_t
typedef struct dt_iop_clipping_data_t
{
float angle; // rotation angle
+ float vert_keystone_amount;
float aspect; // forced aspect ratio
float m[4]; // rot matrix
float ki_h, k_h; // keystone correction, ki and corrected k
@@ -426,6 +457,8 @@ int distort_transform(dt_iop_module_t *self, dt_dev_pixelpipe_iop_t *piece, floa
const float kya = d->kya * ry, kyb = d->kyb * ry, kyc = d->kyc * ry, kyd = d->kyd * ry;
float ma, mb, md, me, mg, mh;
keystone_get_matrix(k_space, kxa, kxb, kxc, kxd, kya, kyb, kyc, kyd, &ma, &mb, &md, &me, &mg, &mh);
+ fprintf(stderr, "ki_h=%.02f, k_h=%.02f, ki_v=%.02f, k_v=%.02f\n",
+ 0.0f, 0.0f, 0.0f, 0.0f);
for(size_t i = 0; i < points_count * 2; i += 2)
{
@@ -1044,6 +1077,7 @@ void cleanup_global(dt_iop_module_so_t *module)
void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pixelpipe_t *pipe,
dt_dev_pixelpipe_iop_t *piece)
{
+
dt_iop_clipping_params_t *p = (dt_iop_clipping_params_t *)p1;
dt_iop_clipping_data_t *d = (dt_iop_clipping_data_t *)piece->data;
@@ -1062,6 +1096,7 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
d->enlarge_x = d->enlarge_y = 0.0f;
d->flip = 0;
d->angle = M_PI / 180.0 * p->angle;
+ d->vert_keystone_amount = p->vert_keystone_amount;
// image flip
d->flags = (p->ch < 0 ? FLAG_FLIP_VERTICAL : 0) | (p->cw < 0 ? FLAG_FLIP_HORIZONTAL : 0);
@@ -1099,6 +1134,10 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
// we adjust the points if the keystoning is not in "full" mode
if(p->k_type == 1) // we want horizontal points to be aligned
{
+ 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);
+
// line equations parameters
float a1 = (d->kxd - d->kxa) / (float)(d->kyd - d->kya);
float b1 = d->kxa - a1 * d->kya;
@@ -1175,6 +1214,7 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
d->kyb = d->kyb - d->kya;
d->kyc = d->kyc - d->kya;
d->kyd = d->kyd - d->kya;
+
keystone_get_matrix(d->k_space, d->kxa, d->kxb, d->kxc, d->kxd, d->kya, d->kyb, d->kyc, d->kyd, &d->a,
&d->b, &d->d, &d->e, &d->g, &d->h);
@@ -1442,7 +1482,7 @@ static void apply_box_aspect(dt_iop_module_t *self, _grab_region_t grab)
void reload_defaults(dt_iop_module_t *self)
{
dt_iop_clipping_params_t tmp
- = (dt_iop_clipping_params_t){ 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 0.0f, 0.0f, 0.2f, 0.2f, 0.8f, 0.2f,
+ = (dt_iop_clipping_params_t){ 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 0.0f, 0.0f, 100.0f, 0.2f, 0.2f, 0.8f, 0.2f,
0.8f, 0.8f, 0.2f, 0.8f, 0, 0, FALSE, TRUE, -1, -1 };
memcpy(self->params, &tmp, sizeof(dt_iop_clipping_params_t));
memcpy(self->default_params, &tmp, sizeof(dt_iop_clipping_params_t));
@@ -1520,6 +1560,15 @@ static void angle_callback(GtkWidget *slider, dt_iop_module_t *self)
commit_box(self, g, p);
}
+static void vert_keystone_amount_callback(GtkWidget *slider, dt_iop_module_t *self)
+{
+ if(self->dt->gui->reset) return;
+ dt_iop_clipping_gui_data_t *g = (dt_iop_clipping_gui_data_t *)self->gui_data;
+ dt_iop_clipping_params_t *p = (dt_iop_clipping_params_t *)self->params;
+ p->vert_keystone_amount = dt_bauhaus_slider_get(slider);
+ commit_box(self, g, p);
+}
+
void gui_reset(struct dt_iop_module_t *self)
{
/* reset aspect preset to default */
@@ -1539,6 +1588,8 @@ static void keystone_type_changed(GtkWidget *combo, dt_iop_module_t *self)
gtk_widget_set_sensitive(g->aspect_presets, TRUE);
return;
}
+ //enable the widget if keystone type is vertical
+ gtk_widget_set_sensitive(g->vert_keystone_amount, which==1);
// we recreate the list to be sure that the "already applied" entry is not display
if(g->k_show == 2)
{
@@ -1599,6 +1650,7 @@ void gui_update(struct dt_iop_module_t *self)
/* update ui elements */
dt_bauhaus_slider_set(g->angle, -p->angle);
+ dt_bauhaus_slider_set(g->vert_keystone_amount, p->vert_keystone_amount);
int hvflip = 0;
if(p->cw < 0)
{
@@ -1876,6 +1928,14 @@ void gui_init(struct dt_iop_module_t *self)
_("right-click and drag a line on the image to drag a straight line"), (char *)NULL);
gtk_box_pack_start(GTK_BOX(self->widget), g->angle, TRUE, TRUE, 0);
+ g->vert_keystone_amount = dt_bauhaus_slider_new_with_range(self, 0.0, 100.0, 0.25, 100.0, 1);
+ dt_bauhaus_widget_set_label(g->vert_keystone_amount, NULL, _("vertical keystone amount"));
+ dt_bauhaus_slider_set_format(g->vert_keystone_amount, "%.02f%%");
+ g_signal_connect(G_OBJECT(g->vert_keystone_amount), "value-changed", G_CALLBACK(vert_keystone_amount_callback), self);
+ g_object_set(G_OBJECT(g->vert_keystone_amount), "tooltip-text",
+ _("set amount of perspective correction"), (char *)NULL);
+ gtk_box_pack_start(GTK_BOX(self->widget), g->vert_keystone_amount, TRUE, TRUE, 0);
+
g->keystone_type = dt_bauhaus_combobox_new(self);
dt_bauhaus_widget_set_label(g->keystone_type, NULL, _("keystone"));
dt_bauhaus_combobox_add(g->keystone_type, _("none"));
--
2.1.4
