Thursday, 20 March 2014

Re: [PATCH 2/2] Truncate caret of text input field at top & bottom boundaries.

Hi Achal-Aggarwal,

Thanks for looking at this.

On 20/03/14 21:21, Achal-Aggarwal wrote:
> ---
> desktop/textarea.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/desktop/textarea.c b/desktop/textarea.c
> index 8e4b1dd..c148a58 100644
> --- a/desktop/textarea.c
> +++ b/desktop/textarea.c
> @@ -462,12 +462,19 @@ static bool textarea_set_caret_internal(struct textarea *ta, int caret_b)
> ((ta->bar_x == NULL) ?
> 0 : SCROLLBAR_WIDTH)
> };
> +
> + /* Truncate height of caret on top and bottom boundaries */
> + height = ta->line_height;
> + if (y - ta->scroll_y + height > cr.y1) {
> + height = cr.y1 - y + ta->scroll_y;
> + }
> +

OK, so you're fixing this by adjusting the height of the caret in the
core. This is not the right approach. The way the caret gets rendered
is the core asks the front end to render a caret, and there's nothing
that says the front end's caret looks the same all the way up.

So for example, if the caret is this shape:

- - - - - - - - - - -
\ /
|
|
|
- -|- - - - - - - - -
|
|
/ \


Then by adjusting the height you ask the front end to render this:

- - - - - - - - - - -
\ /
|
|
/ \
- - - - - - - - - - -

When what you should be doing is letting the front end crop the caret to
the area:

- - - - - - - - - - -
\ /
|
|
|
- - - - - - - - - - -

> msg.ta = ta;
> msg.type = TEXTAREA_MSG_CARET_UPDATE;
> msg.data.caret.type = TEXTAREA_CARET_SET_POS;
> msg.data.caret.pos.x = x - ta->scroll_x;
> msg.data.caret.pos.y = y - ta->scroll_y;
> - msg.data.caret.pos.height = ta->line_height;
> + msg.data.caret.pos.height = height;
> msg.data.caret.pos.clip = &cr;

The clip rectangle gets passed to the front end here. So in the front
end caret rendering code, it should be applying the clip rectangle it
gets given.

It's probably just ignoring the clip rectangle at the moment.

Cheers,

Michael

No comments:

Post a Comment