gmane.editors.lyx.devel

Vincent van Ravesteijn | 5 Oct 16:49

[patch] Selection painting in Insets

This patch solves some problems with painting selections in Insets.
These are:
- multiline selections paint in left margin of the main text (bug 5270),
- the margins to the left and right side of the inset are not correct,
- wrong painting of e.g. a multiline selection of a caption in a float.

I renamed the Row::left_margin_sel back to Row::begin_margin_sel because
of RTL text.

I added a FIXME to BufferView::leftMargin and ::rightMargin that they
are different for RTL texts.

I did some cosmetics (lines <= 80 char, consts).

Vincent

Index: src/BufferView.h
===================================================================
--- src/BufferView.h	(revision 26708)
+++ src/BufferView.h	(working copy)
@@ -92,9 +92,11 @@
 	///
 	void setFullScreen(bool full_screen) { full_screen_ = full_screen; }
 
+	//FIXME: In RTL text this is the left-margin
 	/// right margin
 	int rightMargin() const;
 
+	//FIXME: In RTL text this is the right-margin

(Continue reading)

Pavel Sanda | 5 Oct 18:47

Re: [patch] Selection painting in Insets

Vincent van Ravesteijn wrote:
> This patch solves some problems with painting selections in Insets.
> These are:
> - multiline selections paint in left margin of the main text (bug 5270),
> - the margins to the left and right side of the inset are not correct,
> - wrong painting of e.g. a multiline selection of a caption in a float.
> 
> I renamed the Row::left_margin_sel back to Row::begin_margin_sel because
> of RTL text.
> 
> I added a FIXME to BufferView::leftMargin and ::rightMargin that they
> are different for RTL texts.

the compilable version attached. quick test showed that selection inside
boxes are better now.

pavel

diff --git a/src/BufferView.h b/src/BufferView.h
index d418260..585792d 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -92,9 +92,11 @@ public:
 	///
 	void setFullScreen(bool full_screen) { full_screen_ = full_screen; }

+	//FIXME: In RTL text this is the left-margin
 	/// right margin
 	int rightMargin() const;

(Continue reading)

rgheck | 5 Oct 18:53

Re: [patch] Selection painting in Insets

Pavel Sanda wrote:
> diff --git a/src/BufferView.h b/src/BufferView.h
> index d418260..585792d 100644
> --- a/src/BufferView.h
> +++ b/src/BufferView.h
> @@ -92,9 +92,11 @@ public:
>  	///
>  	void setFullScreen(bool full_screen) { full_screen_ = full_screen; }
>  
> +	//FIXME: In RTL text this is the left-margin
>  	/// right margin
>  	int rightMargin() const;
>  
> +	//FIXME: In RTL text this is the right-margin
>  	/// left margin
>  	int leftMargin() const;
>  
>   
Those names should be changed, then, too. Probably beginMargin and 
endMargin, as you did below.

Richard

Abdelrazak Younes | 5 Oct 19:02

Re: [patch] Selection painting in Insets

On 05/10/2008 18:53, rgheck wrote:
> Pavel Sanda wrote:
>> diff --git a/src/BufferView.h b/src/BufferView.h
>> index d418260..585792d 100644
>> --- a/src/BufferView.h
>> +++ b/src/BufferView.h
>> @@ -92,9 +92,11 @@ public:
>>      ///
>>      void setFullScreen(bool full_screen) { full_screen_ = 
>> full_screen; }
>>
>> +    //FIXME: In RTL text this is the left-margin
>>      /// right margin
>>      int rightMargin() const;
>>
>> +    //FIXME: In RTL text this is the right-margin
>>      /// left margin
>>      int leftMargin() const;
>>
> Those names should be changed, then, too. Probably beginMargin and 
> endMargin, as you did below.

I am not sure those FIXMEs are correct. Even for RTL text, left is left 
and right is right because we paint the insets from right to left in any 
case. When this changes we will think about renaming but not until then.

Abdel.

Pavel Sanda | 10 Oct 01:45

Re: [patch] Selection painting in Insets

Abdelrazak Younes wrote:
> On 05/10/2008 18:53, rgheck wrote:
>> Pavel Sanda wrote:
>>> diff --git a/src/BufferView.h b/src/BufferView.h
>>> index d418260..585792d 100644
>>> --- a/src/BufferView.h
>>> +++ b/src/BufferView.h
>>> @@ -92,9 +92,11 @@ public:
>>>      ///
>>>      void setFullScreen(bool full_screen) { full_screen_ = full_screen; }
>>>
>>> +    //FIXME: In RTL text this is the left-margin
>>>      /// right margin
>>>      int rightMargin() const;
>>>
>>> +    //FIXME: In RTL text this is the right-margin
>>>      /// left margin
>>>      int leftMargin() const;
>>>
>> Those names should be changed, then, too. Probably beginMargin and 
>> endMargin, as you did below.
>
> I am not sure those FIXMEs are correct. Even for RTL text, left is left and 
> right is right because we paint the insets from right to left in any case. 
> When this changes we will think about renaming but not until then.

maybe Dov know something about this?
pavel

(Continue reading)

Vincent van Ravesteijn | 12 Oct 21:24

Re: [patch] Selection painting in Insets

Vincent van Ravesteijn wrote:
> This patch solves some problems with painting selections in Insets.
> These are:
> - multiline selections paint in left margin of the main text 
> (bug 5270),
> - the margins to the left and right side of the inset are not correct,
> - wrong painting of e.g. a multiline selection of a caption in a
> float.
>
> I renamed the Row::left_margin_sel back to Row::begin_margin_sel 
> because
> of RTL text.
>

Is there yet a verdict for this patch ?

Vincent

Index: src/Row.cpp
===================================================================
--- src/Row.cpp	(revision 26861)
+++ src/Row.cpp	(working copy)
@@ -29,7 +29,7 @@
 Row::Row()
 	: separator(0), label_hfill(0), x(0),
 	sel_beg(-1), sel_end(-1),
-	left_margin_sel(false), right_margin_sel(false), 
+	begin_margin_sel(false), end_margin_sel(false), 
 	changed_(false), crc_(0), pos_(0), end_(0)

(Continue reading)

Pavel Sanda | 12 Oct 23:53

Re: [patch] Selection painting in Insets

Vincent van Ravesteijn wrote:
> > I renamed the Row::left_margin_sel back to Row::begin_margin_sel 
> > because
> > of RTL text.
> >
> 
> Is there yet a verdict for this patch ?

i tested the last version and from the functional point of view it was ok. but i know
nothing about painting, so somebody else (Abdel?) should have the last word.

pavel

Abdelrazak Younes | 13 Oct 09:01

Re: [patch] Selection painting in Insets

On 12/10/2008 21:24, Vincent van Ravesteijn wrote:
> Vincent van Ravesteijn wrote:
>    
>> This patch solves some problems with painting selections in Insets.
>> These are:
>> - multiline selections paint in left margin of the main text
>> (bug 5270),
>> - the margins to the left and right side of the inset are not correct,
>> - wrong painting of e.g. a multiline selection of a caption in a
>> float.
>>
>> I renamed the Row::left_margin_sel back to Row::begin_margin_sel
>> because
>> of RTL text.
>>
>>      
>
> Is there yet a verdict for this patch ?
>    

+	if (row.end_margin_sel) {
  		if (text_->isRTL(buffer, beg.paragraph())) {
-			int rm = bv_->rightMargin();
-			pi.pain.fillRectangle(x + rm, y1, x2 - rm, y2 - y1, Color_selection);
+			pi.pain.fillRectangle(x + lm, y1, x2 - lm, y2 - y1,
+				Color_selection);
  		} else {
-			int lm = bv_->leftMargin();
-			pi.pain.fillRectangle(x + x2, y1, width() - lm - x2, y2 - y1, Color_selection);
+			pi.pain.fillRectangle(x + x2, y1, width() - rm - x2, y2 - y1,

(Continue reading)

Dov Feldstern | 13 Oct 16:23

Re: [patch] Selection painting in Insets

Abdelrazak Younes wrote:
> On 12/10/2008 21:24, Vincent van Ravesteijn wrote:
>> Vincent van Ravesteijn wrote:
>>   
>>> This patch solves some problems with painting selections in Insets.
>>> These are:
>>> - multiline selections paint in left margin of the main text
>>> (bug 5270),
>>> - the margins to the left and right side of the inset are not correct,
>>> - wrong painting of e.g. a multiline selection of a caption in a
>>> float.
>>>
>>> I renamed the Row::left_margin_sel back to Row::begin_margin_sel
>>> because
>>> of RTL text.
>>>
>>>      
>>
>> Is there yet a verdict for this patch ?
>>    
> 
> +    if (row.end_margin_sel) {
>          if (text_->isRTL(buffer, beg.paragraph())) {
> -            int rm = bv_->rightMargin();
> -            pi.pain.fillRectangle(x + rm, y1, x2 - rm, y2 - y1, 
> Color_selection);
> +            pi.pain.fillRectangle(x + lm, y1, x2 - lm, y2 - y1,
> +                Color_selection);
>          } else {
> -            int lm = bv_->leftMargin();

(Continue reading)

Abdelrazak Younes | 13 Oct 15:38

Re: [patch] Selection painting in Insets

On 13/10/2008 16:23, Dov Feldstern wrote:
> Abdelrazak Younes wrote:
>>
>> +    if (row.end_margin_sel) {
>>          if (text_->isRTL(buffer, beg.paragraph())) {
>> -            int rm = bv_->rightMargin();
>> -            pi.pain.fillRectangle(x + rm, y1, x2 - rm, y2 - y1, 
>> Color_selection);
>> +            pi.pain.fillRectangle(x + lm, y1, x2 - lm, y2 - y1,
>> +                Color_selection);
>>          } else {
>> -            int lm = bv_->leftMargin();
>> -            pi.pain.fillRectangle(x + x2, y1, width() - lm - x2, y2 
>> - y1, Color_selection);
>> +            pi.pain.fillRectangle(x + x2, y1, width() - rm - x2, y2 
>> - y1,
>> +                Color_selection);
>>
>>
>> Are you sure? You just replaced left with right and right with left 
>> here...
>>
>> Otherwise the patch is just renaming and cosmetics so, provided that 
>> you explain the above, I am OK with the patch. Putting Dov in copy so 
>> that he is aware of your activity.
>>
>> Abdel
>
> Hi!

(Continue reading)

Dov Feldstern | 13 Oct 16:35

Re: [patch] Selection painting in Insets

Abdelrazak Younes wrote:
> On 13/10/2008 16:23, Dov Feldstern wrote:
>> Abdelrazak Younes wrote:
>>>
>>> +    if (row.end_margin_sel) {
>>>          if (text_->isRTL(buffer, beg.paragraph())) {
>>> -            int rm = bv_->rightMargin();
>>> -            pi.pain.fillRectangle(x + rm, y1, x2 - rm, y2 - y1, 
>>> Color_selection);
>>> +            pi.pain.fillRectangle(x + lm, y1, x2 - lm, y2 - y1,
>>> +                Color_selection);
>>>          } else {
>>> -            int lm = bv_->leftMargin();
>>> -            pi.pain.fillRectangle(x + x2, y1, width() - lm - x2, y2 
>>> - y1, Color_selection);
>>> +            pi.pain.fillRectangle(x + x2, y1, width() - rm - x2, y2 
>>> - y1,
>>> +                Color_selection);
>>>
>>>
>>> Are you sure? You just replaced left with right and right with left 
>>> here...
>>>
>>> Otherwise the patch is just renaming and cosmetics so, provided that 
>>> you explain the above, I am OK with the patch. Putting Dov in copy so 
>>> that he is aware of your activity.
>>>
>>> Abdel
>>
>> Hi!

(Continue reading)

Vincent van Ravesteijn | 13 Oct 22:11

Re: [patch] Selection painting in Insets

On Mon, 2008-10-13 at 09:01 +0200, Abdelrazak Younes wrote:
> On 12/10/2008 21:24, Vincent van Ravesteijn wrote:
> > Vincent van Ravesteijn wrote:
> >   
> >> This patch solves some problems with painting selections in Insets.
> >> These are:
> >> - multiline selections paint in left margin of the main text
> >> (bug 5270),
> >> - the margins to the left and right side of the inset are not
> correct,
> >> - wrong painting of e.g. a multiline selection of a caption in a
> >> float.
> >>
> >> I renamed the Row::left_margin_sel back to Row::begin_margin_sel
> >> because
> >> of RTL text.
> >>
> >>     
> >
> > Is there yet a verdict for this patch ?
> >   
> 
> +       if (row.end_margin_sel) {
>                 if (text_->isRTL(buffer, beg.paragraph())) {
> -                       int rm = bv_->rightMargin();
> -                       pi.pain.fillRectangle(x + rm, y1, x2 - rm, y2
> - y1, Color_selection);
> +                       pi.pain.fillRectangle(x + lm, y1, x2 - lm, y2
> - y1,
> +                               Color_selection);

(Continue reading)

Abdelrazak Younes | 13 Oct 22:48

Re: [patch] Selection painting in Insets

On 13/10/2008 22:11, Vincent van Ravesteijn wrote:
> The piece of code you showed is indeed just renaming, but I think this
> makes more sense than the way it was before: if the end margin is
> selected, only the left margin is important for RTL text and the right
> margin for LTR text.
>
> By the way, bv_->leftMargin() and bv_->rightMargin() are defined exactly
> the same. However, when I played with these functions I saw that
> altering one of these does something different for RTL text as it does
> for LTR text. That's why I added the FIXME's last time (which lead to
> doubts from your side).
>
> The patch is not only cosmetics as it always does some 'important'
> things: it sets lm and rm to zero, when we are not in the main buffer
> (i.e. Insets don't have margins). Second, the other block for
> row.begin_margin_sel differ in a number of terms.
>    

OK, I missed that part of the patch indeed. Dov confirmed that it is OK 
so, Pavel, if you have it applied locally please commit.

Abdel.

Pavel Sanda | 14 Oct 01:02

Re: [patch] Selection painting in Insets

Abdelrazak Younes wrote:
> OK, I missed that part of the patch indeed. Dov confirmed that it is OK so, 
> Pavel, if you have it applied locally please commit.

even if i don't speak french?
pavel

Andre Poenitz | 14 Oct 01:09

Re: [patch] Selection painting in Insets

On Tue, Oct 14, 2008 at 01:02:24AM +0200, Pavel Sanda wrote:
> Abdelrazak Younes wrote:
> > OK, I missed that part of the patch indeed. Dov confirmed that it is OK so, 
> > Pavel, if you have it applied locally please commit.
> 
> even if i don't speak french?

That's fine as long as you eat whale.

Andre'

Return

Return to gmane.editors.lyx.devel.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane