Created attachment 113988 [details] 3.5 DXF insertion results In version 3.5 it is possible to insert a DXF image from a file, see attached example. I earlier got a correction of a problem regarding export to PDF of such an image, see bug #64400. Now I encounter a problem again when trying to produce a pdf document from a dxf file. This time the majority of the entities in the DXF file is missing on the inserted image. Therefore, the problem has nothing to do with PDF export. Instead the problem seems to be related to the import of an image on DXF-format. I have attached: 1. Dxf file to insert as an image. 2. Results in version 3.5. 3. Results in version 4.4.1.2 The problem is easy to reproduce. Just insert image and select the attached DXF file. Compare the excellent behavior in version 3.5 with the dispappointing appearance in version 4.4.1.2.
Created attachment 113989 [details] 4.4.1.2 DXF file insertion results
Created attachment 113990 [details] Example DXF file used to produce the results Insertion fails in version 4.4.1.2. Insertion succeeds in version 3.5.
The same problematic behavior has been observed both on Redhat 6 and on Windows 7.
Reproducible with LO 4.4.1.2, Win 8.1 WRITER and DRAW do not open/insert it correctly.
Bibisect results from 43all: 8aabf2aee6514311020b855a95a6e44bab3a5b0d is the first bad commit commit 8aabf2aee6514311020b855a95a6e44bab3a5b0d Author: Bjoern Michaelsen <bjoern.michaelsen@canonical.com> Date: Wed Nov 27 09:32:23 2013 +0000 source-hash-0aa9ced531b8d85ad067c1d156a9708eea628d78 Not 100% sure, but the most likely in that range is probably: commit 2e5167528f7566dd9b000e50fc1610b7bf99132a Author: Armin Le Grand <alg@apache.org> AuthorDate: Thu Oct 31 14:43:21 2013 +0000 Commit: Caolán McNamara <caolanm@redhat.com> CommitDate: Tue Nov 5 15:24:18 2013 +0000 Resolves: #i123500# unified Graphic processing to use GraphicPrimitive2D (cherry picked from commit f5d69b2b8b002ca6905496a9d9065ef76b5641d7) Conflicts: sw/source/core/doc/notxtfrm.cxx Change-Id: I1758aadcbe97ece271277378e62300b895421768
Built to check - commit 2e5167528f7566dd9b000e50fc1610b7bf99132a is indeed the culprit
Migrating Whiteboard tags to Keywords: (bibisected) [NinjaEdit]
@Armin: Care to have a look maybe?
Adding Cc: to Armin Le Grand
I have no idea how dxf worked before, but if it worked in LO and not in AOO and it was added in LO only for Writer and not in the general primitive case for GraphicObject, it might have been removed. Would need to search in older versions how dxf was imported/interpreted and add that more globally to GraphicPrimitive decomposition/rendering, that would make dxf work in all apps immediately.
Looks like dxf is converted to metafile and this is not painted well. When breaking it up (set BP at MIN_ACTIONS_FOR_DIALOG in drviews2.cxx due to the Break dialog not working, force to DoImportMarkedMtf) all geometry appears. This means that the import to metafile is pretty good, just the rendering is not. For a long and good quality fix, imort to primitives would be good. For a fast fix, rendering has to be checked.
...and not to forget: The Break() dialog needs to be fixed. Do we have a task for that?
Created Bug 102852 for the BreakDlg problem
** Please read this message in its entirety before responding ** To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/ If the bug is present, please leave a comment that includes the information from Help - About LibreOffice. If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice. Please DO NOT Update the version field Reply via email (please reply directly on the bug tracker) Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from http://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to 'inherited from OOo'; 4b. If the bug was not present in 3.3 - add 'regression' to keyword Feel free to come ask questions or to say hello in our QA chat: https://kiwiirc.com/nextclient/irc.freenode.net/#libreoffice-qa Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team MassPing-UntouchedBug
Dear Tomas, To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/ If the bug is present, please leave a comment that includes the information from Help - About LibreOffice. If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice. Please DO NOT Update the version field Reply via email (please reply directly on the bug tracker) Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to 'inherited from OOo'; 4b. If the bug was not present in 3.3 - add 'regression' to keyword Feel free to come ask questions or to say hello in our QA chat: https://kiwiirc.com/nextclient/irc.freenode.net/#libreoffice-qa Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team MassPing-UntouchedBug
The DXF gets imported by DXF2GDIMetaFile::Convert to a metafile. That has a size of 2,79 x 3,94 or internally { LT=[0 , 0] RB=[7098 , 10000] [7099 x 10001] } with 25958 entries. Ths is interpreted as 100thmm, so the foloating point numbers from the DXF are gone. Problem is that most items have just 2-5 units (100thmm) difference and thus are *tiny*. Example (from dump): <line startx="3148" starty="5658" endx="3145" endy="5658" style="solid" width="0" dashlen="0" dashcount="0" dotlen="0" dotcount="0" distance="0" join="round" cap="butt"/> <line startx="3145" starty="5659" endx="3148" endy="5659" style="solid" width="0" dashlen="0" dashcount="0" dotlen="0" dotcount="0" distance="0" join="round" cap="butt"/> <polyline style="solid" width="0" dashlen="0" dashcount="0" dotlen="0" dotcount="0" distance="0" join="round" cap="butt"> <point x="3179" y="5664"/> <point x="3194" y="5664"/> <point x="3194" y="5665"/> <point x="3189" y="5665"/> <point x="3188" y="5666"/> </polyline> I will try to extract the metafile...
Created attachment 158820 [details] The metafile, extraccted from saved ODG file
Inserting/opening that metafile creates (of course) the same result. First problem is DXF2GDIMetaFile. It should/may be changed to convert to primitives if no save file is needed. If needed, maybe SVG is a good alternative. For a BugFix like this is too much work, though. Still - when breaking up the graphic, geometry is there. Thus, analyzing situation...
There are two break-up scenarios: (a) wmfemfhelper::interpretMetafile used from MetafilePrimitive2D::create2DDecomposition at paint time (b) ImpSdrGDIMetaFileImport used by SdrEditView::DoImportMarkedMtf to convert to SdrObjects While (b) creates 23653 SdrObjects which do show the geometry (astounding with the low integer data with very small sizes) closer looking reveals the bad quality. Exporting to PDF also. Still, there is data to be seen. Indeed (a) also imports and creates all the data as primitives, but for *some* reason these do not get painted. It may be - tryDrawPolygonStrokePrimitive2DDirect uses DrawPolyLineDirect and that OutputDevice::DrawPolyLine and that OutputDevice::DrawPolyLineDirect and that sets bPixelSnapHairline which makes e.g. WinSalGraphicsImpl::drawPolyLine execute impPixelSnap. That again does transform to discrete (pixel) space, snaps, and converts back. For small objects, all point coordinates get the same for mentioned small distances - There may be other reasons, need to investigate (hard with a metafile with 25000 actions :-)
Indeed bPixelSnapHairline leads to matching coordinates, but when supressing it still no geometry drawn - sigh...
After trying massive and many changes, debugging deep and wide, I started to doubt that std::deque now used in primitive2d::Primitive2DContainer works - and tried for(size_t nAction(0); nAction < nCount && rTargetHolders.Current().size() < 5000; nAction++) in implInterpretMetafile to limit the number of created primitives - and see there - it works! WTF... Also tried in-between in BaseProcessor2D::process other loops: - temp copy in vetor: does not work... const std::vector<primitive2d::Primitive2DReference> aTemp(rSource.begin(), rSource.end()); - loop using 'for(const primitive2d::Primitive2DReference& rrRef : rSource)': does not work... Thus the motor block parts shown *are* the last parts of the 23653 primitives being created - sigh. This was changted to std::deque in commit cd47cf36ebb70a3d6897b0abbb0b3cd8b290e1af. It was a std::deque before as I could see, but since this was moved to new files it is hard to check who and when and why changed to std::deque. Since I ninitially developed that stuff I am pretty sure it was not a std::deque initially... Anyways, of course std::deque should work. It is still possible that it's an error in one of the in-between stages, e.g. wmfemfhelper::TargetHolder uses a std::vector< std::unique_ptr<drawinglayer::primitive2d::BasePrimitive2D> > aTargets; as temporary creation target. Of course it is not acceptable that any std::vector like stl construct holds less than 23653 entries - but now how to find the reason that stuff is lost in-between somewhere...?
Tried with binary, works with 25401, breaks with 25402 when using const size_t nMax(16384 + 5000 + 2500 + 1250 + /*625 + 310*/ + 155 + 77 + /*38*/ + 19 + 10 + 5 + /*2*/ + 1); for(size_t nAction(0); nAction < nCount && rTargetHolders.Current().size() < nMax; nAction++) in implInterpretMetafile as limit for created primitives. Tried next to check for if(!xReference.is()) in BaseProcessor2D::process to see if there are empty slots in these 26599 primitives - no, all are interpreted as set. This means that for the visible motor parts, these get returned multiple times. There are hints in https://en.cppreference.com/w/cpp/container/deque: "As opposed to std::vector, the elements of a deque are not stored contiguously: typical implementations use a sequence of individually allocated fixed-size arrays, with additional bookkeeping, which means indexed access to deque must perform two pointer dereferences, compared to vector's indexed access which performs only one." Thus an error in std::deque acessing the wrong partial arays seems a possibility...
@Noel, @Stephan, any comment regarding Armin's observations in comment 21 and comment 22 ?
It is extraordinarily unlikely that std::deque has a bug. The STL is incredibly broadly tested. For the record, I did the conversion to std::deque, because it showed perf increases when drawing very complex diagrams, since (a) we use that particular list as an append-only data structure (b) we almost never index into it, rather using iterators over it which is exactly the use-case for std::deque. Possibly we could switch to importing these as FieldUnit::PIXEL instead of FieldUnit::MM_100TH, and then set a custom DPI on the image to get the right physical size. That would allow us to use the full range of our integer values.
Just tried (not too complicated) to go back to std::vector - and it's the same. That's good - I would've been very nervous seeing such a bug in stl - sigh... Still - what happens on that 25401 to 25402 mark...? This may get interesting... @Noel: Better change DL to double/transformations, I did some already in AW080 and this may be good stuff for some Tenders (will be too much for one...)
Debugged and debugged, no error found yet. But when commenting out line 584 in drawinglayer\source\tools\wmfemfhelper.cxx // createFillPrimitive(basegfx::B2DPolyPolygon(rPolygon), rTarget, rProperties); which creates element 25402 all lines are shown. Of course filled polys are missing (only a view), but why does creating drawinglayer::primitive2d::PolyPolygonColorPrimitive2D change the behaviour? Poly in question is <polygon> <point x="47" y="9953"/> <point x="7051" y="9953"/> <point x="7051" y="47"/> <point x="47" y="47"/> <point x="47" y="9953"/> </polygon> which has nothing special...
Tried to create PolyPolygonGradientPrimitive2D instead of PolyPolygonColorPrimitive2D, but (after switching off skia under Win which needs ages to draw gradients) no diff - still missing stuff - no idea yet
Oh boy - sometimes your mind is just so blocked... After having debugged all those variants of RefCounting/uniquePtr's and related and finding no error it stroke me like a flash (in a moment far away from my machine, btw...): This has nothing to do with objects/primitives being lost/wrong accessed. It's much simpler - the object 25402 *is* as follows: <polygon> <point x="0" y="10000"/> <point x="7098" y="10000"/> <point x="7098" y="0"/> <point x="0" y="0"/> <point x="0" y="10000"/> </polygon> with the last pen defining action having been (very early) <fillcolor color="#ffffff"/> which is interpreted as white. Thus a rect in dxf-import-page-size gets painted filled white -> all stuff painted before is overpainted. It's a geometrical problem, not an obj-lifetime/mem-or-whatever one. So why is this different in Breaking object? Because the color in the MetaFillColorAction is not 'white' as dumped above, but 0xffffffff which equals the (old) COL_TRANSPARENT definition. Thus, OutputDevice switches *off* filling polygons when this gets set (see OutputDevice::SetFillColor). It works due to DXF-to-metafile converter using a 'real' but unused-for-paint VDev instance just for holding states which does that while the primitive creator uses local states independent from OutputDevice stuff - to be more future-proof - and misses this. Sigh. Those small, grown details... I was tricked by (a) the dump of metafile saying #ffffff and (b) the sheer size of the file in processing, so was hard to see/find. Now it's easy, doing the fix...
Very good work Armin!
Armin Le Grand committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/5b14a18f4748c7b09b432cfae5577c966213a80b tdf#89901 corectly interpret COL_TRANSPARENT It will be available in 7.0.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Verified in Version: 7.0.0.0.alpha0+ Build ID: 5a94ac9eec4a63708262b2389aa2a434aa47112e CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3; Locale: en-US (en_US.UTF-8); UI-Language: en-US Calc: threaded @Armin, thanks for fixing this issue!!
Armin Le Grand committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/4f8325dbcb63627997289889a377a4893e03fcf1 tdf#89901 corectly interpret COL_TRANSPARENT It will be available in 6.4.4. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.