Bug 115100 - Assertion failed, if I set vertical align for gluepoint
Summary: Assertion failed, if I set vertical align for gluepoint
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha0+
Hardware: x86-64 (AMD64) Windows (All)
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:6.1.0 target:6.0.1
Keywords: haveBacktrace
Depends on:
Blocks: Assert
  Show dependency treegraph
 
Reported: 2018-01-18 20:04 UTC by Regina Henschel
Modified: 2018-01-22 11:47 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Output from VS 2017 at breakpoint after "wiederholen" at assertion. (29.40 KB, text/plain)
2018-01-18 20:04 UTC, Regina Henschel
Details
bt with debug symbols (6.76 KB, text/plain)
2018-01-19 20:03 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2018-01-18 20:04:15 UTC
Created attachment 139204 [details]
Output from VS 2017 at breakpoint after "wiederholen" at assertion.

Draw a shape and insert a gluepoint, save and reload.
Click on shape, click on Gluepoint-icon in toolbar, open the "Glue Points" toolbar and undock it.
Select your gluepoint. Click on %-icon to deselect it.
Click on "Vertical align top"-icon.
--> Assertion failed.
Program: C:\LO_build\core\instdir\program\svxcorelo.dll
File: C:\LO_build\core\include\svx/svdglue.hxx
Line: 102
Expression: (nAlg & static_cast<SdrAlign>(0xff00)) == SdrAlign::NONE

I have set a break point in line 102. The attachment contains the output I have got from VS 2017.

I have used Version: 6.1.0.0.alpha0+ (x64)
Build ID: b97a0df0f3234b4c1140ba1418d4b96a592afa4a
CPU threads: 8; OS: Windows 10.0; UI render: default; 
Locale: de-DE (de_DE); Calc: CL

Please consider, that gluepoints might not work in your 5.4, see bug 115089.
Comment 1 Julien Nabet 2018-01-19 20:03:01 UTC
Created attachment 139225 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today + gen rendering, I could reproduce the assert.
I attached bt + some gdb session
Comment 2 Julien Nabet 2018-01-19 20:29:28 UTC
Noel: the asserts have been added with https://cgit.freedesktop.org/libreoffice/core/commit/?id=d66f266cf24d09c2ceb9320f1355ba27114187c2
"convert SDR*ALIGN constants to scoped enum"

void SetHorzAlign(SdrAlign nAlg)                { assert((nAlg & static_cast<SdrAlign>(0xff)) == SdrAlign::NONE); nAlign = SdrAlign(nAlign & static_cast<SdrAlign>(0xFF00)) | (nAlg & static_cast<SdrAlign>(0x00FF)); }

void SetVertAlign(SdrAlign nAlg)                { assert((nAlg & static_cast<SdrAlign>(0xff00)) == SdrAlign::NONE); nAlign = SdrAlign(nAlign & static_cast<SdrAlign>(0x00FF)) | (nAlg & static_cast<SdrAlign>(0xFF00)); }

51  enum class SdrAlign
52  {
53      NONE          = 0x0000,
54      HORZ_CENTER   = 0x0000,
55      HORZ_LEFT     = 0x0001,
56      HORZ_RIGHT    = 0x0002,
57      HORZ_DONTCARE = 0x0010,
58      VERT_CENTER   = 0x0000,
59      VERT_TOP      = 0x0100,
60      VERT_BOTTOM   = 0x0200,
61      VERT_DONTCARE = 0x1000,
62  };

If we take the example of SetVertAlign (but there's the same pb with SetHorzAlign):
assert((nAlg & static_cast<SdrAlign>(0xff00)) == SdrAlign::NONE)
it means the method should work only if we call it with HORZ enum.

I thought about 2 possibilities:
a) assert((nAlg & static_cast<SdrAlign>(0xff00)) != SdrAlign::NONE)
we want an align which includes a vert part. (whatever horizontal part)
b) assert((nAlg & static_cast<SdrAlign>(0x00ff)) == SdrAlign::NONE)
we don't want SetVertAlign includes an horizontal part.

I think b) should be right one, any opinion?
Comment 3 Noel Grandin 2018-01-20 07:14:25 UTC
those asserts are the wrong way around.

    SetHorzAlign 

should be

    assert((nAlg & static_cast<SdrAlign>(0xff00)) == SdrAlign::NONE)

and SetVertAlign

  assert((nAlg & static_cast<SdrAlign>(0xff)) == SdrAlign::NONE)
Comment 4 Julien Nabet 2018-01-20 08:04:25 UTC
I submitted a patch to review here:
https://gerrit.libreoffice.org/#/c/48235/
Comment 5 Commit Notification 2018-01-20 11:12:47 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=619a06d2712e340e82dfd701a0c6363058929c27

tdf#115100: fix assertions in svdglue.hxx

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Julien Nabet 2018-01-22 06:28:14 UTC
I cherry-picked the patch for 6.0, waiting for review, here :
https://gerrit.libreoffice.org/#/c/48263/
Comment 7 Commit Notification 2018-01-22 11:47:41 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a3c053172da5979c8a03fd43f58fc04abbfaf776&h=libreoffice-6-0

tdf#115100: fix assertions in svdglue.hxx

It will be available in 6.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.