Summary: | [Minimap] Problem with alternative colour scheme and widget misalignment | ||
---|---|---|---|
Product: | [Applications] kate | Reporter: | Emmanuel Lepage Vallée <emmanuel.lepage> |
Component: | application | Assignee: | KWrite Developers <kwrite-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | kare.sars |
Priority: | NOR | ||
Version: | Git | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/ktexteditor/9c0df13721838c2e395f94c413529d170ef96163 | Version Fixed In: | |
Sentry Crash Report: | |||
Attachments: |
Synced progressbar prototype
Antialiased rounded rects A new patch with designer inputs |
Description
Emmanuel Lepage Vallée
2012-11-04 20:31:43 UTC
Thanks for the report. This feature is just a week old and we get bug reports. It looks like it is a wanted feature :) #1: (Colors) Should be fixed in trunk. The shield colors are now taken from the palette and the lightness is adjusted according to the foreground/background lightness. #2: (Alignment) The text component has a border that the custom scrollbar does not have in MiniMap mode. This must be investigated/fixed eventually #Mockup: -Rounded corners is a nice idea -Extending the border of the text area around the scrollbar would be nice, but is not worth the effort at the moment. It would mean that the scrollbar and the text area would have to synchronize the the painting... not a trivial task... Patches are welcome ;) #Scaling up will stretch the pixmap and that might not look so good, but the option is there already. I just need to change the limit. There is still a hard limit of ~100 characters per line for performance reasons /Kåre Created attachment 75036 [details]
Synced progressbar prototype
Synced progressbar prototype
>Extending the border of the text area around the scrollbar would be nice, but is not worth the effort at
>the moment. It would mean that the scrollbar and the text area would have to synchronize the the
> painting... not a trivial task... Patches are welcome ;)
Why not! The code is attached as a prototype to this message. It is actually quite simple to sync them, a little harder to display the border correctly.
The sync trick is to add an event filter to the viewPort() widget looking for focus and hover in and out. This will give you the infos you need to modify the paintEvent to draw the correct state.
For the border painting part, I honestly had no idea, so I took code from qframe.cpp and adapted it to run on a QScrollBar. It miss the animation part, it is not very fluid compared to a real border fade animation. I have no idea how to fix that one. What do you think?
I did not implement the bacground color and the hide border on the left because it does not apply to your mini map implementation. I also committed to draw the scrollbar content to keep the code simple.
Created attachment 75048 [details] Antialiased rounded rects This simple patch enable antialiasing for the rounder rect and add a 1px margin on the left and right of the rect. The margin make it feel like the rect is part of the map instead of cutting it in 2. It also make the rounded part separated from the map border, preventing the perception both side are not symmetric. I would also like to point that you have 2 rects drawing on top of each other. I am not so sure what's the point of this. It does create quite a few glitch http://ompldr.org/vZzV0eQ . If there is a purpose, please tell me. The Y mismatch is present on all files, but greater in longer files. You use KMag to spot them. Thanks, I have added tried to fix the alignment issue. (still a bit off at the bottom) 2 rects: if you have a really large file the visible area becomes much smaler than the slider -> draw the slider and the visible area separately. Y mismatch: It is not always easy to get it right ;) I have tried now with master and I can't notice it. The bigest problem with the synced focus is that I would need to also modify the text view and that is not something I'm too eager on doing right now ;) Regards, Kåre Hi,
>The bigest problem with the synced focus is that I would need to also modify the text view and that is not something I'm too eager on doing right now ;)
-Last time I talk about that, but as I shown in the prototype attached to this bug, it does not require any modifications as focus tracking is a core QtGui feature. The only modification is hiding the right border, there is few way to do this without too much code. But there's still the bug in my prototype where the fadein animation is not disaplay. As long as that's not solved, it's a no go.
-Alignment (katepart vs scrollbar) seem to be fixed. There is an extra pixel on the bottom, but it is mostly invisible.
-Y align for the 2 rect is still a little bit off (only visible when using KMag to track it) but only my 1 px. What about disabling the second rect when the whole view fit in a single page?
-What about supporting color option for the background color and rect color (keep your as default)? If I was willing to implement it, could I just go ahead and commit it? I don't want to step on your feet while you are working on this.
-Thanks for merging the anti-aliasing part of the patch I posted, but what about the other change, is there a rational why it is bad or something? Oxygen have a 2px margin for scrollbar for the same rationals as I talked earlier.
Still, thanks for working on this. Having these "modern" features in Kate is really, really cool! I complain a lot on details, I am aware of it, I just want this to be as awesome as it can get. Enough to be willing to put my code where my mouth is (and I don't do that very often).
> --- Comment #6 from Emmanuel Lepage Vallée <elv1313@gmail.com> --- > Hi, > > >The bigest problem with the synced focus is that I would need to also > >modify the text view and that is not something I'm too eager on doing > >right now ;) > -Last time I talk about that, but as I shown in the prototype attached to > this bug, it does not require any modifications as focus tracking is a core > QtGui feature. The only modification is hiding the right border, there is > few way to do this without too much code. But there's still the bug in my > prototype where the fadein animation is not disaplay. As long as that's not > solved, it's a no go. The thing I'm not too eager about is the hiding of the right part of the focus for the KateView. > > -Alignment (katepart vs scrollbar) seem to be fixed. There is an extra pixel > on the bottom, but it is mostly invisible. > > -Y align for the 2 rect is still a little bit off (only visible when using > KMag to track it) but only my 1 px. What about disabling the second rect > when the whole view fit in a single page? The ting is how do I detect this reliably..., Hmm maybe if the slider is more than 2 pixels bigger.. I'll check it out. > > -What about supporting color option for the background color and rect color > (keep your as default)? If I was willing to implement it, could I just go > ahead and commit it? I don't want to step on your feet while you are > working on this. I would like to avoid adding that option. Kate already contains quite a bunch of different colors to configure. The colors are now taken from the theme and light adjusted for the used foreground/background colors. If it seems broken for many we might re-think it. > > -Thanks for merging the anti-aliasing part of the patch I posted, but what > about the other change, is there a rational why it is bad or something? > Oxygen have a 2px margin for scrollbar for the same rationals as I talked > earlier. I re-read your bug report after I had committed the anti-aliasing. I will apply the margin when I get to it :) I think 5 pixels rounding might be a bit too much... > > Still, thanks for working on this. Having these "modern" features in Kate is > really, really cool! I complain a lot on details, I am aware of it, I just > want this to be as awesome as it can get. Enough to be willing to put my > code where my mouth is (and I don't do that very often). I think it is great that so many want this feature and are willing to give feedback and even patches :) /Kåre The problems I have with the colours used for background and foreground is inconsistency. Example: http://ompldr.org/vZzgzcA In this example, I have to point a few things: *The slider background is black for C++ and red for Lua. Why? Because the color order for those 2 languages is not the same. So alert is first in Lua and last in C++. So, in the end, the foreground is totally arbitrary. But I love that red! It may be an accident, but I like it. Too bad only lua have "alert" as first colour... *The "unseen area" background is also different. It create a few problem of it's own: ------For C++, it ain't so bad. I guess you based your math on that one. For Lua however, it is about as bright as the "content" so the brightness differential index is so low that the brain have to focus to see the lines. At least mine do. ------In both care the brightness index of the scrollbar is much higher then the code area. This is problematic for many reasons. (it is the opposite in brighter palette, and in that case, it ain't so bad) ------------First that is high contrast is usually bad for the eye because switching focus between bright and dark area on a screen too often cause the pigment that allow us to see in the dark to be "burned" to turn out vision into day light one. But when focusing back on the dark area, our eye rush to create those things again. Doing that too often will hurt the eye night vision permanently. http://en.wikipedia.org/wiki/Night_vision#Biological_night_vision ------------Second, more related to usability, is the reason why professional multimedia application use dark colours palette. It allow to eye to quickly focus on content, like video editing focus on the video itself. On pure bright application, eye is overwhelmed by light, so it focus on the application rather than part of it. By creating out of control brightness differential between the text canvas and scrollbar, you accidentally create such a focus hub. While the problem is not present with Oxygen default color palette and while looking at C++, the current color model does not scale beyond those default settings. My opinion would be to use the palette().base().color() or Kate current text background colour as background and using use a brightness index averaging the document brightness index on the "mini-lines" to make sure the scrollbar is not distracting. And for the slider background, just adding a config option, the current one is just as random as it can get. It also void the palette in both dark and bright mode (in birght/light mode, bitghter is "on top" while darker is "below", see the normal scrollbar for an example of that.). But then, it may be a good idea to have a little bigger pool of opinions. Created attachment 93802 [details]
A new patch with designer inputs
New patch
Looks good, I'm not 100% sure about the transparency level of the slider handle and the fading of "invisible" area tho ;) You can commit from my point of view. We can tweak the levels later if needed. Git commit 9c0df13721838c2e395f94c413529d170ef96163 by Kåre Särs, on behalf of Emmanuel Lepage Vallee. Committed on 19/08/2015 at 07:26. Pushed by sars into branch 'master'. minimap: Attempt to improve the look and feel This patch attempt to make the minimap look better. * The design has been reviewed during Akademy * It now feel "native" in the canvas for both Breeze and Fusion styles. Oxygen has some hardcoded extra borders, this is a known limitation of this patch. * Improve support for darker themes. The previous implementation failed to properly display the visible area * Partially degrade the visible section view when the handle is at its minimum height. This could be improved later on. M +54 -34 src/view/kateviewhelpers.cpp http://commits.kde.org/ktexteditor/9c0df13721838c2e395f94c413529d170ef96163 |