Differential D10722

other preload microoptimizations
ClosedPublic
Actions

Authored by mart on Feb 21 2018, 5:01 PM.

Details

Reviewers
davidedmundson
broulik
Group Reviewers
Plasma
Commits
R242:79fb4ebfa77f: other preload microoptimizations
Summary

make svgitem schedule a polish right when it's done loading

ScrollView now forces to layout items as soon is created, to preload items
before the window is shown

remove useless include

Test Plan

possible to preload (almost) the whole systray with less hacks
on that part (see D10692)

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
arcpatch-D10722
Lint
No Linters Available
Unit
No Unit Test Coverage
mart created this revision.Feb 21 2018, 5:01 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptFeb 21 2018, 5:01 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
mart requested review of this revision.Feb 21 2018, 5:01 PM
broulik added a subscriber: broulik.Feb 21 2018, 5:03 PM
broulik added inline comments.
src/declarativeimports/core/iconitem.h
42 ↗(On Diff #27707)

QQuickItem should do that for you already

src/declarativeimports/core/svgitem.h
41 ↗(On Diff #27707)

Same here

src/declarativeimports/plasmaextracomponents/qml/ScrollArea.qml
58

typeof contentItem.forceLayout === "function"

davidedmundson added a subscriber: davidedmundson.Feb 21 2018, 5:22 PM
davidedmundson added inline comments.
src/declarativeimports/core/svgitem.cpp
50 ↗(On Diff #27707)

I don't see how this makes any difference.

Every setSomeProperty already calls scheduleImageUpdate regardless of whether we're complete or not.
and if you don't have any properties set, then there's nothing to polish.

mart added inline comments.Feb 22 2018, 2:25 PM
src/declarativeimports/core/svgitem.cpp
50 ↗(On Diff #27707)

when the component is complete, in many cases i guessthe properties would all be already set? (could add a check on that) could even be worth directly a loadPixmap, like an immediate updatePolish() there

davidedmundson requested changes to this revision.Feb 22 2018, 4:36 PM
davidedmundson added inline comments.
src/declarativeimports/core/svgitem.cpp
50 ↗(On Diff #27707)

when the component is complete, in many cases i guessthe properties would all be already set?

We know for definitely they will be already set.

I don't understand what problem you think this is solving.

This revision now requires changes to proceed.Feb 22 2018, 4:36 PM
mart updated this revision to Diff 27795.Feb 22 2018, 4:38 PM
  • Revert most of it
  • only leave the forcelayout part
mart added inline comments.Feb 22 2018, 4:39 PM
src/declarativeimports/core/svgitem.cpp
50 ↗(On Diff #27707)

i was probably just confused reading hotspot reports with and without...
i think all this part can be ignored, i want to leave just the part about preloading of lists

mart updated this revision to Diff 27796.Feb 22 2018, 4:40 PM
  • check for function
mart updated this revision to Diff 27797.Feb 22 2018, 4:41 PM
  • fix typo
mart updated this revision to Diff 27798.Feb 22 2018, 4:41 PM
  • err forgot typeof
broulik accepted this revision.Feb 22 2018, 5:17 PM
This revision was not accepted when it landed; it landed in state Needs Review.Feb 22 2018, 5:42 PM
Closed by commit R242:79fb4ebfa77f: other preload microoptimizations (authored by mart). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 27796

View Options

src/declarativeimports/plasmaextracomponents/qml/ScrollArea.qml

Loading...