Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
 This cleans up InsetGraphics::Cache and renames it as GraphicsInset so that 
 the external inset can use it too.

What do you need old_ascent_ for?

+   old_ascent_ = (type == IMAGE) ? loader_.image()-getHeight() : 50;
+
+   dim.asc = old_ascent_;
+   dim.des = 0;
+   [...]
+   dim_ = dim;

You could use  dim_.asc  instead...

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)


Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Angus Leeming
Andre Poenitz wrote:

 On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
 This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
 that the external inset can use it too.
 
 What do you need old_ascent_ for?
 
 +   old_ascent_ = (type == IMAGE) ? loader_.image()-getHeight() : 50;
 +
 +   dim.asc = old_ascent_;
 +   dim.des = 0;
 +   [...]
 +   dim_ = dim;
 
 You could use  dim_.asc  instead...

This is what I call Juergen magic. I didn't really understand it then. I 
still don't. If you tell me that it is premature optimization then I will 
remove it.

-- 
Angus



Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Wed, Jun 04, 2003 at 08:49:55AM +, Angus Leeming wrote:
 Andre Poenitz wrote:
 
  On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
  This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
  that the external inset can use it too.
  
  What do you need old_ascent_ for?
  
  +   old_ascent_ = (type == IMAGE) ? loader_.image()-getHeight() : 50;
  +
  +   dim.asc = old_ascent_;
  +   dim.des = 0;
  +   [...]
  +   dim_ = dim;
  
  You could use  dim_.asc  instead...
 
 This is what I call Juergen magic. I didn't really understand it then. I 
 still don't. If you tell me that it is premature optimization then I will 
 remove it.

No, I tell you that we nowadays cache all parts of the metrics in dim_,
so there is no need to cache the ascent twice (once in dim_, and once in
old_ascent_).

Partially my doing, in fact...

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)


Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Angus Leeming
Angus Leeming wrote:

 Andre Poenitz wrote:
 
 On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
 This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
 that the external inset can use it too.
 
 What do you need old_ascent_ for?
 
 +   old_ascent_ = (type == IMAGE) ? loader_.image()-getHeight() :
 50; +
 +   dim.asc = old_ascent_;
 +   dim.des = 0;
 +   [...]
 +   dim_ = dim;
 
 You could use  dim_.asc  instead...
 
 This is what I call Juergen magic. I didn't really understand it then. I
 still don't. If you tell me that it is premature optimization then I
 will remove it.

Ok, the question becomes:
can dim_.asc change between it being set in GraphicInset::metrics and it 
being used in GraphicInset::draw? That is what this piece of code is 
testing for. I think not but I don't know whether there is a gap between 
the metrics call and the draw call 
(ie does control passes through the underlying gui process event loop or 
do we have code like metrics(...); draw(...); ?)

Can you provide some insight?

I do remember that Juergen thought it important that the loading process is 
asynchronous, but I'm hazy as to why.

-- 
Angus



Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Wed, Jun 04, 2003 at 09:17:19AM +, Angus Leeming wrote:
 Ok, the question becomes:
 can dim_.asc change between it being set in GraphicInset::metrics and it 
 being used in GraphicInset::draw?

I don't think so.

 That is what this piece of code is testing for. I think not but I don't
 know whether there is a gap between the metrics call and the draw call
 (ie does control passes through the underlying gui process event loop
 or do we have code like metrics(...); draw(...); ?)

The code is like metrics(...); draw(...) .

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)


Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
> This cleans up InsetGraphics::Cache and renames it as GraphicsInset so that 
> the external inset can use it too.

What do you need old_ascent_ for?

+   old_ascent_ = (type == IMAGE) ? loader_.image()->getHeight() : 50;
+
+   dim.asc = old_ascent_;
+   dim.des = 0;
+   [...]
+   dim_ = dim;

You could use  dim_.asc  instead...

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)


Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Angus Leeming
Andre Poenitz wrote:

> On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
>> This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
>> that the external inset can use it too.
> 
> What do you need old_ascent_ for?
> 
> +   old_ascent_ = (type == IMAGE) ? loader_.image()->getHeight() : 50;
> +
> +   dim.asc = old_ascent_;
> +   dim.des = 0;
> +   [...]
> +   dim_ = dim;
> 
> You could use  dim_.asc  instead...

This is what I call "Juergen magic". I didn't really understand it then. I 
still don't. If you tell me that it is "premature optimization" then I will 
remove it.

-- 
Angus



Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Wed, Jun 04, 2003 at 08:49:55AM +, Angus Leeming wrote:
> Andre Poenitz wrote:
> 
> > On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
> >> This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
> >> that the external inset can use it too.
> > 
> > What do you need old_ascent_ for?
> > 
> > +   old_ascent_ = (type == IMAGE) ? loader_.image()->getHeight() : 50;
> > +
> > +   dim.asc = old_ascent_;
> > +   dim.des = 0;
> > +   [...]
> > +   dim_ = dim;
> > 
> > You could use  dim_.asc  instead...
> 
> This is what I call "Juergen magic". I didn't really understand it then. I 
> still don't. If you tell me that it is "premature optimization" then I will 
> remove it.

No, I tell you that we nowadays cache all parts of the metrics in dim_,
so there is no need to cache the ascent twice (once in dim_, and once in
old_ascent_).

Partially my doing, in fact...

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)


Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Angus Leeming
Angus Leeming wrote:

> Andre Poenitz wrote:
> 
>> On Tue, Jun 03, 2003 at 06:17:10PM +, Angus Leeming wrote:
>>> This cleans up InsetGraphics::Cache and renames it as GraphicsInset so
>>> that the external inset can use it too.
>> 
>> What do you need old_ascent_ for?
>> 
>> +   old_ascent_ = (type == IMAGE) ? loader_.image()->getHeight() :
>> 50; +
>> +   dim.asc = old_ascent_;
>> +   dim.des = 0;
>> +   [...]
>> +   dim_ = dim;
>> 
>> You could use  dim_.asc  instead...
> 
> This is what I call "Juergen magic". I didn't really understand it then. I
> still don't. If you tell me that it is "premature optimization" then I
> will remove it.

Ok, the question becomes:
can dim_.asc change between it being set in GraphicInset::metrics and it 
being used in GraphicInset::draw? That is what this piece of code is 
testing for. I think not but I don't know whether there is a gap between 
the metrics call and the draw call 
(ie does control passes through the underlying gui "process event" loop or 
do we have code like metrics(...); draw(...); ?)

Can you provide some insight?

I do remember that Juergen thought it important that the loading process is 
asynchronous, but I'm hazy as to why.

-- 
Angus



Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all

2003-06-04 Thread Andre Poenitz
On Wed, Jun 04, 2003 at 09:17:19AM +, Angus Leeming wrote:
> Ok, the question becomes:
> can dim_.asc change between it being set in GraphicInset::metrics and it 
> being used in GraphicInset::draw?

I don't think so.

> That is what this piece of code is testing for. I think not but I don't
> know whether there is a gap between the metrics call and the draw call
> (ie does control passes through the underlying gui "process event" loop
> or do we have code like metrics(...); draw(...); ?)

The code is like metrics(...); draw(...) .

Andre'

-- 
Those who desire to give up Freedom in order to gain Security, will not have,
nor do they deserve, either one. (T. Jefferson or B. Franklin or both...)