vcl/source/window/menu.cxx            |   59 +++++++++++++-----------------
 vcl/win/gdi/salnativewidgets-luna.cxx |   65 ++++++----------------------------
 2 files changed, 39 insertions(+), 85 deletions(-)

New commits:
commit 1be170d0629cf761f0ee4173007a3c021966546e
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Mon Jan 3 19:38:53 2022 +0300
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Jan 4 08:44:09 2022 +0100

    tdf#146551: improve native checkmark and bullet drawing on Windows
    
    This takes marks' borders into account, and removes some hacks used
    previously in place of those.
    
    It introduces one new hack - adjusting only rectangle's top in
    ImplPaintCheckBackground - to workaround one pixel less height of
    selected images background on Windows, caused by the current method
    of drawing that background, abusing toolbar button for that on all
    platforms. Ideally, drawing it (or the whole image) should be
    implemented using OutputDevice::DrawNativeControl with dedicated
    ControlPart value, which would enable use of ImplDrawTheme with
    MCB_BITMAP menu background state on Windows.
    
    I have tested on Windows 10 (using both default and Skia drawing,
    with normal and scaled UI), and on Ubuntu with gtk3, x11, and kf5
    plugins (since common code is affected); it seems to give expected
    results on all of them.
    
    Change-Id: I677c3af30fedc608dbc6b391d5dbbbdb141da0d3
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127909
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>

diff --git a/vcl/source/window/menu.cxx b/vcl/source/window/menu.cxx
index e671341a8292..6af85523f8a1 100644
--- a/vcl/source/window/menu.cxx
+++ b/vcl/source/window/menu.cxx
@@ -1331,8 +1331,8 @@ Size 
Menu::ImplGetNativeCheckAndRadioSize(vcl::RenderContext const & rRenderCont
                                               aCtrlRegion, 
ControlState::ENABLED, aVal,
                                               aNativeBounds, aNativeContent))
             {
-                rCheckHeight = aNativeBounds.GetHeight();
-                nCheckWidth = aNativeContent.GetWidth();
+                rCheckHeight = aNativeBounds.GetHeight() - 1;
+                nCheckWidth = aNativeContent.GetWidth() - 1;
             }
         }
         if (rRenderContext.IsNativeControlSupported(ControlType::MenuPopup, 
ControlPart::MenuItemRadioMark))
@@ -1341,8 +1341,8 @@ Size 
Menu::ImplGetNativeCheckAndRadioSize(vcl::RenderContext const & rRenderCont
                                                       aCtrlRegion, 
ControlState::ENABLED, aVal,
                                                       aNativeBounds, 
aNativeContent))
             {
-                rRadioHeight = aNativeBounds.GetHeight();
-                nRadioWidth = aNativeContent.GetWidth();
+                rRadioHeight = aNativeBounds.GetHeight() - 1;
+                nRadioWidth = aNativeContent.GetWidth() - 1;
             }
         }
     }
@@ -1410,11 +1410,11 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
 
     tools::Long nMinMenuItemHeight = nFontHeight;
     tools::Long nCheckHeight = 0, nRadioHeight = 0;
-    Size aMaxSize = ImplGetNativeCheckAndRadioSize(*pWin->GetOutDev(), 
nCheckHeight, nRadioHeight); // FIXME
-    if( aMaxSize.Height() > nMinMenuItemHeight )
-        nMinMenuItemHeight = aMaxSize.Height();
+    Size aMarkSize = ImplGetNativeCheckAndRadioSize(*pWin->GetOutDev(), 
nCheckHeight, nRadioHeight);
+    if( aMarkSize.Height() > nMinMenuItemHeight )
+        nMinMenuItemHeight = aMarkSize.Height();
 
-    Size aMaxImgSz;
+    tools::Long aMaxImgWidth = 0;
 
     const StyleSettings& rSettings = pWin->GetSettings().GetStyleSettings();
     if ( rSettings.GetUseImagesInMenus() )
@@ -1431,8 +1431,8 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
                )
             {
                 Size aImgSz = pData->aImage.GetSizePixel();
-                if ( aImgSz.Height() > aMaxImgSz.Height() )
-                    aMaxImgSz.setHeight( aImgSz.Height() );
+                if ( aImgSz.Width() > aMaxImgWidth )
+                    aMaxImgWidth = aImgSz.Width();
                 if ( aImgSz.Height() > nMinMenuItemHeight )
                     nMinMenuItemHeight = aImgSz.Height();
                 break;
@@ -1441,7 +1441,6 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
     }
 
     Size aSz;
-    tools::Long nCheckWidth = 0;
     tools::Long nMaxWidth = 0;
 
     for ( size_t n = pItemList->size(); n; )
@@ -1464,25 +1463,23 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
             // Image:
             if (!IsMenuBar() && ((pData->eType == MenuItemType::IMAGE) || 
(pData->eType == MenuItemType::STRINGIMAGE)))
             {
-                Size aImgSz = pData->aImage.GetSizePixel();
+                tools::Long aImgHeight = pData->aImage.GetSizePixel().Height();
 
-                aImgSz.AdjustHeight(4 ); // add a border for native marks
-                aImgSz.AdjustWidth(4 ); // add a border for native marks
-                if ( aImgSz.Width() > aMaxImgSz.Width() )
-                    aMaxImgSz.setWidth( aImgSz.Width() );
-                if ( aImgSz.Height() > aMaxImgSz.Height() )
-                    aMaxImgSz.setHeight( aImgSz.Height() );
-                if ( aImgSz.Height() > pData->aSz.Height() )
-                    pData->aSz.setHeight( aImgSz.Height() );
+                aImgHeight += 4; // add a border for native marks
+                if (aImgHeight > pData->aSz.Height())
+                    pData->aSz.setHeight(aImgHeight);
             }
 
             // Check Buttons:
             if (!IsMenuBar() && pData->HasCheck())
             {
-                nCheckWidth = aMaxSize.Width();
                 // checks / images take the same place
                 if( ( pData->eType != MenuItemType::IMAGE ) && ( pData->eType 
!= MenuItemType::STRINGIMAGE ) )
-                    nWidth += nCheckWidth + nExtra * 2;
+                {
+                    nWidth += aMarkSize.Width() + nExtra * 2;
+                    if (aMarkSize.Height() > pData->aSz.Height())
+                        pData->aSz.setHeight(aMarkSize.Height());
+                }
             }
 
             // Text:
@@ -1490,7 +1487,7 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
             {
                 const SalLayoutGlyphs* pGlyphs = 
pData->GetTextGlyphs(pWin->GetOutDev());
                 tools::Long nTextWidth = 
pWin->GetOutDev()->GetCtrlTextWidth(pData->aText, pGlyphs);
-                tools::Long nTextHeight = pWin->GetTextHeight();
+                tools::Long nTextHeight = pWin->GetTextHeight() + 
EXTRAITEMHEIGHT;
 
                 if (IsMenuBar())
                 {
@@ -1524,8 +1521,6 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
                 pData->aSz.setHeight( std::max( std::max( nFontHeight, 
pData->aSz.Height() ), nMinMenuItemHeight ) );
             }
 
-            pData->aSz.AdjustHeight(EXTRAITEMHEIGHT ); // little bit more 
distance
-
             if (!IsMenuBar())
                 aSz.AdjustHeight(pData->aSz.Height() );
 
@@ -1573,11 +1568,11 @@ Size Menu::ImplCalcSize( vcl::Window* pWin )
         sal_uInt16 gfxExtra = static_cast<sal_uInt16>(std::max( nExtra, 
tools::Long(7) )); // #107710# increase space between checkmarks/images/text
         nImgOrChkPos = static_cast<sal_uInt16>(nExtra);
         tools::Long nImgOrChkWidth = 0;
-        if( aMaxSize.Height() > 0 ) // NWF case
-            nImgOrChkWidth = aMaxSize.Height() + nExtra;
+        if( aMarkSize.Height() > 0 ) // NWF case
+            nImgOrChkWidth = aMarkSize.Height() + nExtra;
         else // non NWF case
             nImgOrChkWidth = nFontHeight/2 + gfxExtra;
-        nImgOrChkWidth = std::max( nImgOrChkWidth, aMaxImgSz.Width() + 
gfxExtra );
+        nImgOrChkWidth = std::max( nImgOrChkWidth, aMaxImgWidth + gfxExtra );
         nTextPos = static_cast<sal_uInt16>(nImgOrChkPos + nImgOrChkWidth);
         nTextPos = nTextPos + gfxExtra;
 
@@ -1632,9 +1627,11 @@ static void ImplPaintCheckBackground(vcl::RenderContext 
& rRenderContext, vcl::W
     {
         ImplControlValue    aControlValue;
         aControlValue.setTristateVal(ButtonValue::On);
+        tools::Rectangle r = i_rRect;
+        r.AdjustTop(1);
 
         bNativeOk = rRenderContext.DrawNativeControl(ControlType::Toolbar, 
ControlPart::Button,
-                                                     i_rRect,
+                                                     r,
                                                      ControlState::PRESSED | 
ControlState::ENABLED,
                                                      aControlValue,
                                                      OUString());
@@ -1824,10 +1821,6 @@ void Menu::ImplPaint(vcl::RenderContext& rRenderContext, 
Size const & rSize,
 
                 tools::Rectangle aOuterCheckRect(Point(aPos.X()+nImgOrChkPos, 
aPos.Y()),
                                           Size(pData->aSz.Height(), 
pData->aSz.Height()));
-                aOuterCheckRect.AdjustLeft(1 );
-                aOuterCheckRect.AdjustRight( -1 );
-                aOuterCheckRect.AdjustTop(1 );
-                aOuterCheckRect.AdjustBottom( -1 );
 
                 // CheckMark
                 if (!bLayout && !IsMenuBar() && pData->HasCheck())
diff --git a/vcl/win/gdi/salnativewidgets-luna.cxx 
b/vcl/win/gdi/salnativewidgets-luna.cxx
index a8d508c9b44d..6a21a3d6d394 100644
--- a/vcl/win/gdi/salnativewidgets-luna.cxx
+++ b/vcl/win/gdi/salnativewidgets-luna.cxx
@@ -365,46 +365,9 @@ static void impl_drawAeroToolbar( HDC hDC, RECT rc, bool 
bHorizontal )
     }
 }
 
-/**
- * Gives the actual rectangle used for rendering by ControlType::MenuPopup's
- * ControlPart::MenuItemCheckMark or ControlPart::MenuItemRadioMark.
- */
-static tools::Rectangle GetMenuPopupMarkRegion(const ImplControlValue& rValue)
-{
-    tools::Rectangle aRet;
-
-    auto pMVal = dynamic_cast<const MenupopupValue*>(&rValue);
-    if (!pMVal)
-        return aRet;
-
-    aRet.SetTop(pMVal->maItemRect.Top());
-    aRet.SetBottom(pMVal->maItemRect.Bottom() + 1); // see below in 
drawNativeControl
-    if (AllSettings::GetLayoutRTL())
-    {
-        aRet.SetRight(pMVal->maItemRect.Right() + 1);
-        aRet.SetLeft(aRet.Right() - (pMVal->getNumericVal() - 
pMVal->maItemRect.Left()));
-    }
-    else
-    {
-        aRet.SetRight(pMVal->getNumericVal());
-        aRet.SetLeft(pMVal->maItemRect.Left());
-    }
-
-    return aRet;
-}
-
 static bool implDrawNativeMenuMark(HDC hDC, HTHEME hTheme, RECT rc, 
ControlPart nPart,
-                                   ControlState nState, const 
ImplControlValue& aValue,
-                                   OUString const& aCaption)
+                                   ControlState nState, OUString const& 
aCaption)
 {
-    if (aValue.getType() == ControlType::MenuPopup)
-    {
-        tools::Rectangle aRectangle = GetMenuPopupMarkRegion(aValue);
-        rc.top = aRectangle.Top();
-        rc.left = aRectangle.Left();
-        rc.bottom = aRectangle.Bottom();
-        rc.right = aRectangle.Right();
-    }
     int iState = (nState & ControlState::ENABLED) ? MCB_NORMAL : MCB_DISABLED;
     ImplDrawTheme(hTheme, hDC, MENU_POPUPCHECKBACKGROUND, iState, rc, 
aCaption);
     if (nPart == ControlPart::MenuItemCheckMark)
@@ -1015,7 +978,7 @@ static bool ImplDrawNativeControl( HDC hDC, HTHEME hTheme, 
RECT rc,
             else if( nPart == ControlPart::MenuItemCheckMark || nPart == 
ControlPart::MenuItemRadioMark )
             {
                 if (nState & ControlState::PRESSED)
-                    return implDrawNativeMenuMark(hDC, hTheme, rc, nPart, 
nState, aValue, aCaption);
+                    return implDrawNativeMenuMark(hDC, hTheme, rc, nPart, 
nState, aCaption);
                 else
                     return true; // unchecked: do nothing
             }
@@ -1072,16 +1035,6 @@ bool WinSalGraphics::drawNativeControl( ControlType 
nType,
         }
     }
 
-    if (pImpl && nType == ControlType::MenuPopup && (nPart == 
ControlPart::MenuItemCheckMark || nPart == ControlPart::MenuItemRadioMark))
-    {
-        tools::Rectangle aRectangle = GetMenuPopupMarkRegion(aValue);
-        if (!aRectangle.IsEmpty())
-        {
-            cacheRect = buttonRect = aRectangle;
-            keySize = cacheRect.GetSize();
-        }
-    }
-
 
     ControlCacheKey aControlCacheKey(nType, nPart, nState, keySize);
     if (pImpl != nullptr && 
pImpl->TryRenderCachedNativeControl(aControlCacheKey, buttonRect.Left(), 
buttonRect.Top()))
@@ -1348,10 +1301,18 @@ bool WinSalGraphics::getNativeControlRegion(  
ControlType nType,
                     MENU_POPUPCHECK,
                     MC_CHECKMARKNORMAL,
                     aBoxRect ) );
-                if( aBoxRect.GetWidth() && aBoxRect.GetHeight() )
+                if (!aRect.IsEmpty())
                 {
-                    rNativeContentRegion = aRect;
-                    rNativeBoundingRegion = rNativeContentRegion;
+                    if (MARGINS mg;
+                        SUCCEEDED(GetThemeMargins(hTheme, hDC, 
MENU_POPUPCHECK, MC_CHECKMARKNORMAL,
+                                                  TMT_CONTENTMARGINS, nullptr, 
&mg)))
+                    {
+                        aRect.AdjustLeft(-mg.cxLeftWidth);
+                        aRect.AdjustRight(mg.cxRightWidth);
+                        aRect.AdjustTop(-mg.cyTopHeight);
+                        aRect.AdjustBottom(mg.cyBottomHeight);
+                    }
+                    rNativeContentRegion = rNativeBoundingRegion = aRect;
                     bRet = true;
                 }
             }

Reply via email to