Bug 142567

Summary: [WMF/EMF] RestoreDC should read and use 'SavedDC' value
Product: LibreOffice Reporter: Valek Filippov <frob>
Component: graphics stackAssignee: Bartosz <gang65>
Status: RESOLVED FIXED    
Severity: normal CC: gang65
Priority: medium    
Version: unspecified   
Hardware: All   
OS: All   
Whiteboard: target:7.2.0
Crash report or crash signature: Regression By:
Bug Depends on:    
Bug Blocks: 59814, 103859    
Attachments: EMF sample with SOME RestoreDC using "-2" for 'SavedDC' value
How it should look like
WMF sample with RestoreDC other than "nSavedDc == -1"
Screenshot of WMF sample opened in LO 7.2 and MS Paint side by side
EMF with SavedDC set to 0
EMF sample with 'SavedDC' set to 3

Description Valek Filippov 2021-05-30 15:02:54 UTC
Description:
[MS-EMF] 2.3.1.16 says:
"SavedDC (4 bytes): A signed integer that specifies the saved state to restore relative to the current state. This value MUST be negative; –1 represents the state that was most recently saved on the stack, –2 the one before that, etc."

LO simply pops the context once.
https://github.com/LibreOffice/core/blob/master/emfio/source/reader/emfreader.cxx#L1103:L1107


Steps to Reproduce:
Open attached EMF sample.

Actual Results:
It shows empty object.

Expected Results:
It should show red rectangle.


Reproducible: Always


User Profile Reset: No



Additional Info:
The issue is isolated from tdf#59814.
LO doesn't handle the stack of the context properly, as a result the combination of ModifyWorldTransform records places the rectangle outside of the visible area.

In the original bug that leads to shifts of some shape positions.
Comment 1 Valek Filippov 2021-05-30 15:04:58 UTC
Created attachment 172454 [details]
EMF sample with SOME RestoreDC using "-2" for 'SavedDC' value
Comment 2 Valek Filippov 2021-05-30 15:13:26 UTC
Implementation in WMF has the same problem.
However, implementation is not the same.

From [MS-WMF] 2.3.5.10:
"nSavedDC (2 bytes): A 16-bit signed integer that defines the saved state to be restored. If this member is positive, nSavedDC represents a specific instance of the state to be restored. If this member is negative, nSavedDC represents an instance relative to the current state."

I will try to make a sample to demonstrate a problem for both: negative below -1 and positive values.

Unfortunately it's unclear if in case of WMF newer contexts are preserved or disposed after restoring to the older one in the stack.
Ability to point out to the specific context could allow re-entrance to the previously used context (eg. switch back and forth between two contexts w/o setting the values again for the "newer" one).
Comment 3 Valek Filippov 2021-05-30 15:16:20 UTC
Created attachment 172455 [details]
How it should look like
Comment 4 Valek Filippov 2021-05-30 21:10:02 UTC
Created attachment 172461 [details]
WMF sample with RestoreDC other than "nSavedDc == -1"

It should look like red/blue/red rectangles. LO shows it as three green rectangles.

Observations:
1. RestoreDC with nSavedDC==0 is doing nothing in WMF. (LO currently pops the context.)
2. If context is switched to the bottom of the stack, then all jumped other contexts seem to become unavailable -- switching to one of them using positive nSavedDC has not effect.
Comment 5 Valek Filippov 2021-05-30 21:13:43 UTC
Created attachment 172462 [details]
Screenshot of WMF sample opened in LO 7.2 and MS Paint side by side
Comment 6 Valek Filippov 2021-05-30 23:58:53 UTC
Created attachment 172463 [details]
EMF with SavedDC set to 0
Comment 7 Valek Filippov 2021-05-31 00:00:21 UTC
Created attachment 172464 [details]
EMF sample with 'SavedDC' set to 3

In EMF all non-negative values for SavedDC in RestoreDC EMR seems to be handled the same as "-1" value.
Comment 8 Commit Notification 2021-06-11 10:32:56 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/cabb9f5c8e22a23a453559636d9c9b3c2b0a5984

WMF/EMF tdf#59814 tdf#142567 Fix RestoreDC record

It will be available in 7.2.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.
Comment 9 Commit Notification 2021-06-11 16:42:09 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8d3e7b2f5836106eac5172d8f4868bb540d652e6

EMF tdf#59814 tdf#142567 Align RestoreDC record with MSO implementation

It will be available in 7.2.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.