Re: [patch]: clean-up InsetGraphics::Cache and make it accessible to all
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
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
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
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
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
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
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
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
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
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...)