D27505: Do not crash when icon's width or height is 0

[Arjen Hiemstra]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:fcae16b30b06: Do not crash when icon's width or 
height is 0 (authored by ahiemstra).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27505?vs=76002&id=76043

REVISION DETAIL
  https://phabricator.kde.org/D27505

AFFECTED FILES
  src/icon.cpp

To: ahiemstra, #kirigami, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, apol, ahiemstra, mart, hein

D27505: Do not crash when icon's width or height is 0

[Arjen Hiemstra]

ahiemstra added a comment.


  In D27505#614240 , @davidedmundson 
wrote:
  
  > Would be good to dump this test qml somewhere in the repo. A folder full of 
tests which just have one component in a bunch of configurations.
  >  I've been pushing for that in p-f, and it catches so many things.
  
  
  Actually, it should be possible to test this as part of the autotests. 
However, I just found out that the Kirigami autotests are broken and have not 
run for a pretty long time. So I will make a separate patch for that.

REPOSITORY
  R169 Kirigami

BRANCH
  icon_crash

REVISION DETAIL
  https://phabricator.kde.org/D27505

To: ahiemstra, #kirigami, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, apol, ahiemstra, mart, hein

D27505: Do not crash when icon's width or height is 0

[Arjen Hiemstra]

ahiemstra added a comment.


  It most likely does, since Icon also has no implicit width/height it will 
default to 0 and crash when there's no special layout properties specified.

REPOSITORY
  R169 Kirigami

BRANCH
  icon_crash

REVISION DETAIL
  https://phabricator.kde.org/D27505

To: ahiemstra, #kirigami, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, apol, ahiemstra, mart, hein

D27505: Do not crash when icon's width or height is 0

[Nathaniel Graham]

ngraham added a comment.


  Does this fix https://bugs.kde.org/show_bug.cgi?id=417844?

REPOSITORY
  R169 Kirigami

BRANCH
  icon_crash

REVISION DETAIL
  https://phabricator.kde.org/D27505

To: ahiemstra, #kirigami, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, apol, ahiemstra, mart, hein

D27505: Do not crash when icon's width or height is 0

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  Weird. plasma-framework's version of icon (which this is copied from) has had 
that check for years.
  
  Would be good to dump this test qml somewhere in the repo. A folder full of 
tests which just have one component in a bunch of configurations.
  I've been pushing for that in p-f, and it catches so many things.

REPOSITORY
  R169 Kirigami

BRANCH
  icon_crash

REVISION DETAIL
  https://phabricator.kde.org/D27505

To: ahiemstra, #kirigami, davidedmundson
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, mart, hein

D27505: Do not crash when icon's width or height is 0

[Arjen Hiemstra]

ahiemstra created this revision.
ahiemstra added a reviewer: Kirigami.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
ahiemstra requested review of this revision.

REVISION SUMMARY
  When Icon's width or height is 0, I get a crash with the following backtrace:
  
#0  0x77b9cd30 in QSGTexture::setFiltering(QSGTexture::Filtering) 
() from /usr/lib/libQt5Quick.so.5
#1  0x77bd439c in 
QSGOpaqueTextureMaterialShader::updateState(QSGMaterialShader::RenderState 
const&, QSGMaterial*, QSGMaterial*) () from /usr/lib/libQt5Quick.so.5
#2  0x77bb7857 in 
QSGBatchRenderer::Renderer::renderMergedBatch(QSGBatchRenderer::Batch const*) 
() from /usr/lib/libQt5Quick.so.5
#3  0x77bbd5b6 in QSGBatchRenderer::Renderer::renderBatches() () 
from /usr/lib/libQt5Quick.so.5
#4  0x77bbdcc5 in QSGBatchRenderer::Renderer::render() () from 
/usr/lib/libQt5Quick.so.5
#5  0x77ba1832 in QSGRenderer::renderScene(QSGBindable const&) () 
from /usr/lib/libQt5Quick.so.5
#6  0x77ba1d14 in QSGRenderer::renderScene(unsigned int) () from 
/usr/lib/libQt5Quick.so.5
#7  0x77c10713 in 
QSGDefaultRenderContext::renderNextFrame(QSGRenderer*, unsigned int) () from 
/usr/lib/libQt5Quick.so.5
#8  0x77c7b3a1 in QQuickWindowPrivate::renderSceneGraph(QSize 
const&, QSize const&) () from /usr/lib/libQt5Quick.so.5 
  
  This patch prevents Icon from doing anything when width or height is 0, 
avoiding the crash.

TEST PLAN
  The following QML code, when ran through qmlscene, no longer crashes:
  
import QtQuick 2.12
import org.kde.kirigami 2.11 as Kirigami
Item { 
Kirigami.Icon { source: "document-new" }
}

REPOSITORY
  R169 Kirigami

BRANCH
  icon_crash

REVISION DETAIL
  https://phabricator.kde.org/D27505

AFFECTED FILES
  src/icon.cpp

To: ahiemstra, #kirigami
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein