Differential D8688

Create annotation color from revision hash, not RNG number
ClosedPublic
Actions

Authored by kossebau on Nov 7 2017, 12:08 AM.

Details

Reviewers
brauch
Group Reviewers
KDevelop
Summary

Generate the same background colors for annotations on every invocation of the
annotation bar, for consistent display.

Also generate the brush objects per revision only on-demand, no need to
pregenerate all of them, as often the user only looks at the annotations for
a smaller range.

Also fix the hash algorithm for VcsRevision class to generate proper hash
values for all types. Before it would have generated a value of 0 for any
string-based revision values, like e.g. used by the git vcs plugin.

Diff Detail

Repository
R32 KDevelop
Branch
reproducableannotationcolor
Lint
No Linters Available
Unit
No Unit Test Coverage
kossebau created this revision.Nov 7 2017, 12:08 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptNov 7 2017, 12:08 AM
brauch accepted this revision.Nov 7 2017, 12:10 AM
brauch added a subscriber: brauch.

Looks good, thanks!

kdevplatform/vcs/vcsrevision.cpp
179 ↗(On Diff #22010)

can you simply return qHash<int>(revisionValue.value())?

This revision is now accepted and ready to land.Nov 7 2017, 12:10 AM
kossebau added a comment.Nov 7 2017, 12:10 AM

Could be actually three separate commits. Would split them, if agreed.

(patches are small fixes in preparation for the bigger delegate-based annotationbar rendering rework coming hopefully later this week)

kossebau added inline comments.Nov 7 2017, 12:18 AM
kdevplatform/vcs/vcsrevision.cpp
179 ↗(On Diff #22010)

Might be possible. Going via qHash(QString) was a helpless (and untested) try to have non-small integers, so that the bits used for the annotation color are not a null.

But I have to admit I have no clear idea when such special revisions actually pop-up, and if they ever would be delivered for the annotations... guess I should add a TODO at least?

brauch added inline comments.Nov 7 2017, 12:20 AM
kdevplatform/vcs/vcsrevision.cpp
179 ↗(On Diff #22010)

You hash the int anyways. I think qHash(3) is something with completely (predictively, of course) random bits, or at least it should be.

kossebau added inline comments.Nov 7 2017, 12:24 AM
kdevplatform/vcs/vcsrevision.cpp
179 ↗(On Diff #22010)

Ah, sorry, my eyes are already closing being almost asleep, read you proposed just return revisionValue.value().
Okay, then indeed something I agree to, will update.

kossebau updated this revision to Diff 22013.Nov 7 2017, 12:45 AM
  • do qHash directly on VcsRevision::RevisionSpecialType enum value instead of

string

  • also do qHash for revisionValue.toULongLong(), now that I found one needs to

explicitly use ::qHash to avoid the compiler trying to cast qHash(int-like)
to qHash(const KDevelop::VcsRevision& rev)

Revision Contents
Changeset List

Diff 22013

View Options

kdevplatform/vcs/models/vcsannotationmodel.cpp

Loading...
View Options

kdevplatform/vcs/vcsrevision.cpp

Loading...