Hi Frank,
eventually found some time to revisit this yesterday & today :-)
I will try to combine responses to the last two emails from you to
explain what I have done ( hope this doesn't confuse things more )
On Mon, 2008-07-28 at 13:38 +0200, Frank Schönheit - Sun Microsystems
Germany wrote:
> - setting a non-linked URL at multiple controls at once doesn't work.
> This is why the PropertyHandler API distinguishes between
> ObtainedValue and Success in the onInteractivePropertySelection
> method. The former is used to just return the value, which is then
> forwarded to all components which are being inspected.
>
> You should simply spare setting the ImageURL in
> impl_browseForImage_nothrow, and instead put the ObjectID into
> _out_rNewValue.
done, seems to work nicely, thanks for the hint
>
> + the documentation of css.graphic.GraphicObjectAccess/XGraphicObject is
> *very* sparse
didn't yet do that so please ignore it's absence in the rework
> - 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)
now mentioned in the com.sun.star.graphic.MediaDescriptor.idl
> + GObjectAccessImpl ctor should also check for sId being empty
done
>
> - /me thinks the form component handler should perhaps just use
> m_eComponentClass, instead of the SupportNonLinkedImage setting in the
> component context
done
> + in toolkit: the handling of a Graphic-URL would belong into
> lcl_getGraphicFromURL_nothrow
done, I made it a member of the ImageProducer class ( because it needs
to set/reset a member variable ) I removed the "lcl_" prefix and now it
is called getGraphicFromURL_nothrow, hope that is ok
> + in goodies: IMO, the handling of the Graphic-URL would belong into a
> dedicated method. Reasoning: a) "implLoadMemory" suggests the method
> handles memory URLs, and b) all other URL schemes have dedicated
> methods, too (implLoadResource, implLoadRepositoryImage,
> implLoadStandardImage), so this really should also hold for some
> implLoadGraphics
done
>
> + goodies/source/unographic/graphicunoaccess.cxx does not compile with
> warnings=errors. Ts.
not done yet so please also ignore this
>
> + 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
I made it empty
>
> + 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
> - close the document, discarding any changes
> => crash
fixed ( as suspected it is to do with the GraphicObject scheme 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
> (non-product builds only)
seems to be to do with creating a thumbnail see attached trace if you
are interested.
> - close the document
> - open the document, again
> => three assertions about problems with the graphics
> (non-product builds only)
I don't get assertions with the new patch
On Wed, 2008-07-30 at 09:57 +0200, Frank Schönheit - Sun Microsystems Germany
wrote:
> 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.
done, but, I also needed to cater for the GraphicObject URL in
StartProduction ( because otherwise it will blank out the URL in the
ImageProducer )
>
>
> 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.
>
My understanding of how GraphicManager & Friends hang together is as
follows ( of course I could be wrong given I am just reading the
code. /me admits to trusting the debugger more than his code reading
skills )
Note: a GraphicObject has a vcl::Graphic member ( and this is where the
XGraphic instance is obtained )
When you create a GraphicObject it registers with the GraphicManager.
The GraphicManager upon receiving the GraphicObject inserts it in its
own list. The GraphicManager in turn also inserts the GraphicObject into
GraphicCache ( the GraphicCache is NOT the display cache but it does
actually manage the display cache ). The GraphicCache manages
GraphicCacheEntry objects, the DisplayCache ( that the Tools|Option|
Memory parameters pertain to ) manages different afaiks objects, each
GraphicCacheEntry contains a list of GraphicObject pointers, where each
GraphicObject has the same id, so these GraphicObjects are used to
reference count the Cache entry. i.e when the last GraphicObject of a
certain ID is destroyed the CacheEntry ( for that ID ) is also
destroyed.
When you create a GraphicObject with a specific ID and there is a cache
entry for that GraphicObject then the vcl::Graphic member is 'filled'
with from a GraphicObject ( with that specific ID ) from the cache.
The GraphicCache doesn't act like a traditional ( or my understanding of
a traditional cache ) e.g. it does not reap any GraphicObjects. Instead
a GraphicObject can be 'Swapped[In|Out]' A swap-out meaning the
GraphicObject's vcl::Graphic member is sent a 'SwapOut' call, this I
guess means the vcl::Graphic releases it's internal state ( and
associated memory ). Conversely a SwapIn call will call SwapIn on the
associated vcl::Graphic object ( which it seems I assume will repopulate
the Graphic from the associated stream (assuming there is one) ). So how
does a SwapOut call happen, it can happen in 2 ways
1) someone calls SwapOut directly on a GraphicObject instance ( note:
only the Graphic for the GraphicObject instance in question is Swapped
out and not for 'all' GraphicObjects with the same ID.
2) a timer fires ( but one has to set the Timer on the GraphicObject )
note for 1 & 2 above we ( who control the GraphicObjects created via
GraphicObjectAccess::CreateGraphicObjectxxxx ) do neither of these
things
a SwapIn call is done when attempting to access the Graphic associated
with the GraphicObject, e.g. when 'filling' the Graphic member of a
GraphicObject just created with an ID corrosponding to an existing
GraphCacheEntry - entry
I hope what I have said above is reasonably accurate
> >> - GraphicObjectAccess sounds like it should be a
> GraphicsCache(Access)
[...]
> 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.
After some thought I changed the servicename ( and idl filename ) to
GraphicObjectFactory
> 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
It would seem there is no mutex locking etc. for the GraphicObject &
friends :-(
> 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.
done
> > you object to renaming the class/method to something like
> > GraphicAccess::getImageXStream,
>
> Fine with me, go ahead
done
> 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?
none whatsover, please lets forget that question. ( I think I got a
little confused with a temporary stream and what might get persisted ;-)
but the stream in question is nothing to do with that )
beyond the 2 issues I didn't do ( idl documention of the XGraphicObject
etc. ) and WAE removal I hope I have addressed all of the things you
mentioned.
anyway, latest version attached to
http://qa.openoffice.org/issues/show_bug.cgi?id=38215
thanks
Noel
Noel
#0 BitmapReadAccess::ImplCreate (this=0xae632230, [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/bmpacc.cxx:78
#1 0xb64fa704 in BitmapReadAccess (this=0xae632230, [EMAIL PROTECTED],
bModify=1 '\001') at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/bmpacc.cxx:50
#2 0xb64fa736 in BitmapWriteAccess (this=0xae632230, [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/bmpacc.cxx:337
#3 0xb64d9844 in Bitmap::AcquireWriteAccess (this=0xbfb31870) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/bitmap.cxx:494
#4 0xb64d9f56 in Bitmap::Erase (this=0xbfb31870, [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/bitmap.cxx:516
#5 0xb651fec3 in GDIMetaFile::ImplBmpMonoFnc ([EMAIL PROTECTED],
pBmpParam=0xbfb31d74) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/gdimtf.cxx:1641
#6 0xb6525c08 in GDIMetaFile::ImplExchangeColors (this=0xbfb31f04,
pFncCol=0xb651f130 <GDIMetaFile::ImplColMonoFnc(Color const&, void const*)>,
pColParam=0xbfb31d78, pFncBmp=0xb651fce8 <GDIMetaFile::ImplBmpMonoFnc(BitmapEx
const&, void const*)>, pBmpParam=0xbfb31d74) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/gdimtf.cxx:1815
#7 0xb652760f in GDIMetaFile::GetMonochromeMtf (this=0xae6344f0, [EMAIL
PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/vcl/source/gdi/gdimtf.cxx:2100
#8 0xb768c5df in GraphicHelper::createThumb_Impl ([EMAIL PROTECTED],
nMaximumExtent=256, [EMAIL PROTECTED], pOverlay=0x0, pOverlayRect=0x0) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/graphhelp.cxx:378
#9 0xb768cd8d in GraphicHelper::getThumbnailFormatFromGDI_Impl
(pMetaFile=0xae6344f0, bSigned=0 '\0', [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/graphhelp.cxx:431
#10 0xb7601e34 in SfxObjectShell::WriteThumbnail (this=0x8d739e8, bEncrypted=0
'\0', bSigned=0 '\0', bIsTemplate=0 '\0', [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objstor.cxx:3798
#11 0xb7602064 in SfxObjectShell::GenerateAndStoreThumbnail (this=0x8d739e8,
bEncrypted=0 '\0', bSigned=0 '\0', bIsTemplate=0 '\0', [EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objstor.cxx:3743
#12 0xb760e8c3 in SfxObjectShell::SaveTo_Impl (this=0x8d739e8, [EMAIL
PROTECTED], pSet=0x0) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objstor.cxx:1590
#13 0xb7611946 in SfxObjectShell::PreDoSaveAs_Impl (this=0x8d739e8, [EMAIL
PROTECTED], [EMAIL PROTECTED], pParams=0x90de750) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objstor.cxx:2999
#14 0xb7612639 in SfxObjectShell::CommonSaveAs_Impl (this=0x8d739e8, [EMAIL
PROTECTED], [EMAIL PROTECTED], aParams=0x8e4e0e8) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objstor.cxx:2855
#15 0xb761fb3c in SfxObjectShell::APISaveAs_Impl (this=0x8d739e8, [EMAIL
PROTECTED], aParams=0x8e4e0e8) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objserv.cxx:357
#16 0xb766b833 in SfxBaseModel::impl_store (this=0x8f3b708, [EMAIL PROTECTED],
[EMAIL PROTECTED], bSaveTo=0 '\0') at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/sfxbasemodel.cxx:2648
#17 0xb767681a in SfxBaseModel::storeAsURL (this=0x8f3b708, [EMAIL PROTECTED],
[EMAIL PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/sfxbasemodel.cxx:1584
#18 0xb7688de1 in SfxStoringHelper::GUIStoreModel (this=0xbfb33360, [EMAIL
PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED], bPreselectPassword=0 '\0',
aUserSelectedName={pData = 0xbfb336f0}) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/guisaveas.cxx:1466
#19 0xb76223e8 in SfxObjectShell::ExecFile_Impl (this=0x8d739e8, [EMAIL
PROTECTED]) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/doc/objserv.cxx:674
#20 0xb7624c63 in SfxStubSfxObjectShellExecFile_Impl (pShell=0x8d739e8, [EMAIL
PROTECTED]) at ../../unxlngi6/inc/sfxslots.hxx:161
#21 0xb77149fa in SfxShell::CallExec (this=0x8d739e8, pFunc=0xb7624c4b
<SfxStubSfxObjectShellExecFile_Impl(SfxShell*, SfxRequest&)>, [EMAIL
PROTECTED]) at ../../inc/sfx2/shell.hxx:204
#22 0xb7729355 in SfxShell::ExecuteSlot (this=0x8d739e8, [EMAIL PROTECTED],
pIF=0x85b4f00) at
/media/disk-3/BUILD-related/LATEST_HEAD/build/dev300-m25/sfx2/source/control/shell.cxx:964
#23 0xadb0d32e in ScTabViewShell::ExecuteSave () from
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
