Hi Noel,
>> actually, I think the usage of SfxMedium there is historically only,
>> dating back to times when images where loaded asynchronously. Not sure
>> if this is still the case today, but given that in a lot of cases, the
>> "real" loading happens internally beforehand, then the bits are copied
>> to a memory stream, and then the SfxMedium loads from the memory
>> stream, this probably is completely obsolete.
> assuming that is the case then probably that leg of the logic needs
> adjusting, I am a little uncomfortable with it just 'falling' into the
> test for the GraphicObject scheme and would prefer a more definitive
> if( sURL.indexOf( rtl::OUString(
> RTL_CONSTASCII_USTRINGPARAM("vnd.sun.star.GraphicObject:") ) == 0 ) ) further
> up in the method
Agreed.
Didn't try it in detail, but I suppose that the lines you added to
StartProduction just need to be moved to SetURL, this omitting all the
medium-related stuff in case of a GraphicObject.
>> (though /me wonders whether this will stop working as soon as the
>> GraphicsCache can be globally disabled)
> are there plans for that?
Not that I know.
I was just wondering, I do not know how the graphic manager works, I
just assume that if you retrieve an ID for an XGraphic, then this ID
will be invalid as soon as the XGraphic (more precise: the last of the
possible multiple XGraphics referring to the image with this ID) dies.
It just tastes a little bit ... error prone in some esoteric (future)
scenarios. Not sure at all.
> but anyway in this usage the standalone
> GraphicObjects will just be inserted into the GraphicManager, I don't
> think the GraphicCache ( display cache ) will come into play
Maybe the graphic cache set up in the UI is indeed completely different
from the graphic manager ... as said, I do not know the caching
strategy/parameters of the graphic manager implementation.
>> - setting a non-linked URL at multiple controls at once doesn't work.
>> ...
> :->> good to know ( would never even have tested it and I didn't realise
> it was possible to do a multiobject propertychange ) But, this will
> however cause a problem because it would mean the xGrfObj ( set in
> impl_browseForImage_nothrow ) will go out of scope before the imageURL
> will get set ( and hence a new XGraphicObject with the same ID created )
> in the toolkit. I will have to look deeper into that for alternatives
You can transfer anything you like in the obtained value. In particular,
you can transfer the XGraphic object itself - the value will later on be
passed to the setPropertyValue method of the property handler (for every
of the image controls, if you have a multiple selection), and it's the
responsibility of this method then to translate it into an image URL, again.
The alternative would be to remove the "composeable" flag from the
ImageURL property, but this is something I would not really like to see.
>> + the documentation of css.graphic.GraphicObjectAccess/XGraphicObject
>> is
>> *very* sparse
> sure, I was only willing to invest zero effort here as creating the uno
> wrapper was
> a) a last minute thing to solve a cyclic link problem ( and I only saw
> the problem when doing a full clean build )
Hey, you make me glad with anything which involves UNO instead of direct
linking :)
>> - GraphicObjectAccess sounds like it should be a GraphicsCache(Access)
>> - if I see this right, then its sole purpose is to convert between an
>> XGraphic and the ID this graphic has in the cache - right?
> not exactly, I really only think of this service as a way to generate
> GraphicObject(s). Regardless of using this uno wrapper of not, when you
> create a GraphicObject ( with an id ) you don't really know whether you
> are creating a new object or just getting a handle to an already managed
> object. When you create a GraphicObject without an id then of course you
> are causing a new object to be managed. But I have no talent for naming
> things, if you can suggest another name you think is more appropriate, I
> will gladly substitute it
Well, naming things (a good way) is always the hardest part :)
Stay with GraphicObjectAccess, your explanations are convincing.
>> - the support of the vnd.sun.star.GraphicObject URL should be
>> documented
>> in MediaProperties service, for the URL property (which also documents
>> (some of) the other supported URL schemes)
> not sure what you mean here, are we affecting MediaProperties somehow?
see css.graphic.MediaProperties, in particular the URL property of this
service. There's already documentation that private:resource and such
are supported (though some of the supported URL schemes are missing,
shame on the implementors), and the GraphicObject URL scheme should also
be mentioned there, IMO.
>> + GObjectAccessImpl is not thread-safe at all, it should have an own
>> mutex
> Will this help any?, I did think about this ( whilst keeping in mind the
> mantra "putting a mutex in any openoffice code is like playing russian
> roulette" )
Hehe :)
> My conclusion was that
> a) the only state mpGObject is never modified ( it is just created )
> putting a mutex around it's access seemed extreme
but you call into mpGObject, and if it is not threadsafe itself (which I
do not know), this can be a problem. For instance, if you have multiple
threads, concurrently calling setGraphic and getGraphic, this results in
concurrent calls to mpGObject->SetGraphic/GetGraphic - which might or
might not yield undesired behavior ...
In general, I (personally - you might want to remember that the goodies
project is not really my domain) prefer having a mutex.
Using mutexes is not really russian roulette, as long as you follow some
rules. Admittedly, those rules are sometimes hard (and expensive) to
follow, and known only after 5+ years of experience with fixing deadlocks :)
> b) ok we call [get|set]XGraphic but we are just a proxy for a real
> object and just forward some calls, if a mutex should go anywhere it
> probably should be in the GraphicObject itself ( but I don't know if I
> would be brave enough to do that given the risk of introducing some
> strange threading foo in this object )
Agreed.
> c) remembering that you could create 1 or more XGraphicObjects with the
> same id then an 'own' mutex wouldn't help much either
The 'own' mutex (in the sense of a member of the GObjectAccessImpl)
would only help to keep this particular instance consistent.
Admittedly, without knowing about the thread-safety/-awareness/-affinity
of the GraphicObject implementation itself, it's hard to tell what kind
of locking policy the GObjectAccessImpl class should have.
In every case, I suggest playing with the non-product version. It will,
for instance, report some use cases of the kind "non-threadsafe code
called without locked solar mutex".
> thinking about the above it seemed more sensible to me that we might
> expect that the users of such a class might be better qualified to make
> sure there was no concurrent access,
I am not sure this is possible. Everything which has an UNO API can be
used from multiple places concurrently - you can pass the instance
around, so even clients on remote machines can use the very same
instance you do in your code.
It's an interesting idea to forbid this by policy - not sure if this is
allowed :) (effectively, it means to prohibit passing around the
interface to the object, except you have complete control over which
code uses it).
Anyway. Personally, I would prefer having the mutex, and fixing the
possible deadlocks later, than not having it, and fixing the possible
crashs later :)
And in this particular case, I'd even say there aren't problems ahead:
Since the implementation does not call into other UNO components,
chances are good that no other mutex will ever be locked.
>> - using ::svt::ImageResourceAccess::getImageXStream for a graphics URL
>> sounds a little bit ... weird. At least it makes (should make) the
>> casual code reader wonder if there's something wrong.
> ahem yes, I think the method here is a little unfortunately named
> because it will resolve any URL the GraphicProvider can manage, would
> you object to renaming the class/method to something like
> GraphicAccess::getImageXStream,
Fine with me, go ahead
> And, this is a place where I did have another question/comment but not
> about the naming but rather about the fact that the images are converted
> to png, basically I was asking was it a problem that all images were
> conferted to png and should I modify the method to allow an extra param
> to be passed, the extra param could be used to indicate to use mime type
> of the xGraphic retrieved from the URL rather than hardcode it to png,
> the it seems I pruned the comment/question :-/ )
IIRC, I introduced this class quite some time ago, I do not know why I
used PNG as hard-coded mime type - probably because I had to use *anything*.
Would there be an advantage in using a flexible mime type?
>> + when an "embedded" graphics is used, then the respective item in the
>> property browser shows the vnd.sun.sttar.GraphicObject URL - which is
>> of no use at all (and confusing) to the user. In such a case, leaving
>> the property display empty, or showing something like "<embedded
>> graphics>" might be better
> absolutely, additionally there is a nice bug where if you select an item
> ( a previously selected one ) in the drop down list for filenames the
> image disappears ( the url looks right but it has been put to
> lower-case, I am guessing it gets passed through some url parsing code
> but I didn't see where and didn't look further yet )
SvtURLBox, respectively FileURLBox, in svtools/source/dont_know_right_now.
>> + bug:
>> - open a new text document
>> - insert an image control
>> - in the property browser, add an image to the control, not linked
>> => the image is displayed in the control
>> - in the property browser, focus and control other than the "Graphics"
>> control
>> - put the focus back to the "Graphics" control
>> - put the focus to some other control, again
>> => the image vanishes from the control, though the vnd.sun.*-URL is
>> still displayed as current "Graphics" URL
> I wonder is this similar to the above, I will see.
I also suppose all of those strange effects have to do with the
SvtURLBox/FileURLBox, which might not be prepared to handle non-file-URLs.
>> + bug:
>> - open a new text document
>> - insert an image control
>> - do not assign any graphics to the image control
>> - save the document
>> => assertion saying something about forbidden acccess to an empty
>> bitmap
> does this happen for linked also?, regardless I will look and try to fix
Didn't try, and already patched out the changes (they were in my working
copy), so can't try right now. I suppose it's something simple as
passing an empty string to an URL conversion function which isn't
prepared to handle this.
>> (non-product builds only)
>> - close the document
>> - open the document, again
>> => three assertions about problems with the graphics
>> (non-product builds only)
> ok, point taken, always build with the dbg util stuff ;-), I will look
> into that also.
thanks.
Ciao
Frank
--
- Frank Schönheit, Software Engineer [EMAIL PROTECTED] -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Base http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
