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 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 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
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 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...)


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

2003-06-04 Thread Angus Leeming
This cleans up InsetGraphics::Cache and renames it as GraphicsInset so that 
the external inset can use it too.

-- 
AngusIndex: src/insets/graphicinset.h
===
RCS file: src/insets/graphicinset.h
diff -N src/insets/graphicinset.h
--- /dev/null	1 Jan 1970 00:00:00 -
+++ src/insets/graphicinset.h	3 Jun 2003 17:13:29 -
@@ -0,0 +1,90 @@
+// -*- C++ -*-
+/**
+ * \file graphicinset.h
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Angus Leeming
+ *
+ * Full author contact details are available in file CREDITS
+ */
+
+#ifndef GRAPHICINSET_H
+#define GRAPHICINSET_H
+
+#include "dimension.h"
+
+#include "graphics/GraphicsLoader.h"
+#include "graphics/GraphicsParams.h"
+#include "graphics/GraphicsTypes.h"
+
+#include 
+#include 
+
+
+class MetricsInfo;
+class PainterInfo;
+
+
+class GraphicInset
+{
+public:
+	GraphicInset();
+	GraphicInset(GraphicInset const &);
+
+	/** Set the message that the inset will show when the
+	 *  display of the graphic is deactivated.
+	 *  The default is nothing, meaning that the inset will
+	 *  show a message descibing the state of the image
+	 *  loading process.
+	 */
+	void setNoDisplayMessage(string const & msg);
+	
+	/// Refresh the info about which file to display and how to display it.
+	void update(grfx::Params const & params);
+	/// File name, image size, rotation angle etc.
+	grfx::Params const & params() const;
+
+	/// compute the size of the object returned in dim
+	void metrics(MetricsInfo & mi, Dimension & dim) const;
+	/// draw inset and update (xo, yo)-cache
+	void draw(PainterInfo & pi, int x, int y) const;
+
+	/// Is the stored checksum different to that of the graphics loader?
+	bool hasFileChanged() const;
+	/// An accessor function to the cached store.
+	BufferView * view() const;
+
+	/** Connect and you'll be informed when the loading status of the image
+	 *  changes.
+	 */
+	typedef boost::signal0::slot_type slot_type;
+	boost::signals::connection connect(slot_type const &) const;
+
+private:
+	enum DisplayType {
+		IMAGE,
+		STATUS_MESSAGE,
+		NODISPLAY_MESSAGE
+	};
+
+	/// Is the image ready to draw, or should we display a message instead?
+	DisplayType displayType() const;
+	
+	/// The message to display instead of the graphic itself.
+	string const statusMessage() const;
+
+	/// The stored data.
+	grfx::Loader loader_;
+	grfx::Params params_;
+	string nodisplay_message_;
+
+	/// These are all cached variables.
+	mutable int old_ascent_;
+	mutable unsigned long checksum_;
+	mutable boost::weak_ptr view_;
+	mutable Dimension dim_;
+};
+
+
+#endif // NOT GRAPHICINSET_H
Index: src/insets/graphicinset.C
===
RCS file: src/insets/graphicinset.C
diff -N src/insets/graphicinset.C
--- /dev/null	1 Jan 1970 00:00:00 -
+++ src/insets/graphicinset.C	3 Jun 2003 17:13:30 -
@@ -0,0 +1,287 @@
+/**
+ * \file graphicinset.C
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Angus Leeming
+ *
+ * Full author contact details are available in file CREDITS
+ */
+
+#include 
+
+#include "insets/graphicinset.h"
+
+#include "buffer.h"
+#include "BufferView.h"
+#include "gettext.h"
+#include "metricsinfo.h"
+
+#include "frontends/font_metrics.h"
+#include "frontends/LyXView.h"
+#include "frontends/Painter.h"
+
+#include "graphics/GraphicsImage.h"
+
+#include "support/filetools.h"
+
+
+GraphicInset::GraphicInset()
+	: old_ascent_(0), checksum_(0)
+{}
+
+
+GraphicInset::GraphicInset(GraphicInset const & other)
+	: loader_(other.loader_),
+	  params_(other.params_),
+	  nodisplay_message_(other.nodisplay_message_),
+	  old_ascent_(0),
+	  checksum_(0)
+{}
+
+
+void GraphicInset::update(grfx::Params const & params)
+{
+	if (!params.filename.empty()) {
+		lyx::Assert(AbsolutePath(params.filename));
+		loader_.reset(params_.filename, params_);
+	}
+	params_ = params;
+}
+
+
+grfx::Params const & GraphicInset::params() const
+{
+	return params_;
+}
+
+
+bool GraphicInset::hasFileChanged() const
+{
+	unsigned long const new_checksum = loader_.checksum();
+	bool const file_has_changed = checksum_ != new_checksum;
+	if (file_has_changed)
+		checksum_ = new_checksum;
+	return file_has_changed;
+}
+
+
+BufferView * GraphicInset::view() const
+{
+	return view_.lock().get();
+}
+
+
+boost::signals::connection GraphicInset::connect(slot_type const & slot) const
+{
+	return loader_.connect(slot);
+}
+
+
+void GraphicInset::setNoDisplayMessage(string const & str)
+{
+	nodisplay_message_ = str;
+}
+
+
+string const GraphicInset::statusMessage() const
+{
+	switch (loader_.status()) {
+		case grfx::WaitingToLoad:
+			return _("Not shown.");
+		case grfx::Loading:
+			return _("Loading...");
+		case grfx::Converting:
+			return _("Converting to loadable format...");
+		case grfx::Loaded:
+			return _("Loaded into memor