sb0 changed the topic of #m-labs to: ARTIQ, Migen, MiSoC, Mixxeo & other M-Labs projects :: fka #milkymist :: Logs http://irclog.whitequark.org/m-labs
bentley` has quit [Ping timeout: 244 seconds]
fengling has joined #m-labs
bentley` has joined #m-labs
sb0 has quit [Quit: Leaving]
fengling has quit [Ping timeout: 240 seconds]
fengling has joined #m-labs
fengling has quit [Ping timeout: 240 seconds]
fengling has joined #m-labs
rjo__ has joined #m-labs
gric__ has joined #m-labs
larsc has quit [*.net *.split]
tmbinc__ has quit [*.net *.split]
stekern has quit [*.net *.split]
gric_ has quit [*.net *.split]
kaaliakahn has quit [*.net *.split]
rjo has quit [*.net *.split]
tmbinc__ has joined #m-labs
stekern has joined #m-labs
kaaliakahn has joined #m-labs
larsc has joined #m-labs
FabM has joined #m-labs
key2 has joined #m-labs
rjo__ is now known as rjo
sb0 has joined #m-labs
<cr1901_modern>
rjo: Wow, you did a really good job cleaning this up
<cr1901_modern>
I pulled, stashed my own changes. Looking over it now to see if I can add/change anything
<cr1901_modern>
rjo: I added a third argument to slider.pixelPosToRangeVal. We didn't hit this bug yet, b/c gr.x() has always been zero, but the mouse pixel position is actually relative to the widget left, while widget.rangeToReal is relative to slider left.
<cr1901_modern>
So far widgetLeft == sliderLeft, but if it didn't, the transform would break. So I added a bool to figure out whether the pixel position is due to the mouse, or due to rangeToReal
<rjo>
cr1901_modern: that should be done the right way by subtracting/adding gr.x() where necessary
<rjo>
cr1901_modern: there is also some handle painting not getting done. could you please look into that?
<rjo>
on unmaximize for me
<cr1901_modern>
rjo: Sure
<cr1901_modern>
re: subtracting gr.x(), that's fine. I can go that route too.
<cr1901_modern>
You're right about it being a better idea; the mouse already requires knowledge about the widget's layout anway
<rjo>
decide on a coordinate system for this, don't booleanize the use of multiple unless absolutely necessary. that would just add code that needs maintaining.
<cr1901_modern>
Okay, then, pixel position 0 is the left of the slider (as is currently)
fengling has quit [Quit: WeeChat 1.4]
<rjo>
you should clean that up. i see no reason why the axis and the proxy need to know about the handle width. let the 0 be the left of the widget == the left of the axis
<rjo>
that means have the axis start at the left of the widget. it already ends at the right of the widget (and not at the right end of the groove).
<rjo>
and correct for gr.x() and the handle width within the slider.
<cr1901_modern>
rjo: So you want me to get rid of the effectiveWidth() function?
<rjo>
cr1901_modern: if that is the result or a side effect, why not? less code for the same effect is always better.
key2 has quit [Ping timeout: 252 seconds]
<cr1901_modern>
code that's not there cannot fail.
<cr1901_modern>
(well it can't do anything, but it especially can't fail)
<cr1901_modern>
rjo: hasTracking() is no longer useful since slider position always equals slider value from Qt's point of view, so I'm going to remove it
<cr1901_modern>
I'm going to remove the check
<cr1901_modern>
Well, let me rephrase- I noticed you left hasTracking in, while deleting the startPos/stopPos variables. Did you realize it was still necessary?
* cr1901_modern
rereads QAbstractSlider. Huh nevermind. It does make a slight difference.
<cr1901_modern>
rjo: Ignore the above
key2 has joined #m-labs
<rjo>
cr1901_modern: no. leave it in. it is for the external interface, not the internal one.
<cr1901_modern>
rjo: Re: the handlewidth. I mathematically proved that I can safely remove handlewidth from the resizeEvent (only works b/c pixelToReal is linear)
<cr1901_modern>
So I'm getting rid of it
<cr1901_modern>
(a + b)/(c + d + e) == a/(c + e) iff b/d == a/(c + e).
<cr1901_modern>
w_handle/f(w_handle) == w_event/f(w_event) can be rewritten as:
<cr1901_modern>
B/c f(x) is linear, we can rewrite as f(w_handle*w_event) == f(w_event*w_handle), which is an identity. QED.
<cr1901_modern>
f(w_event) = f(refRight) - f(0) = f(refRight) by superpos- oh, screw it, it works
<cr1901_modern>
So why doesn't the code work when I remove it? Hrm...
ylamarre has joined #m-labs
<rjo>
it's just an offset
<rjo>
what code?
<cr1901_modern>
The scanwidget
<cr1901_modern>
resizing breaks when I remove handleWidth
<cr1901_modern>
even though it shouldn't
<cr1901_modern>
Actually, hold that thought
ylamarre has quit [Quit: ylamarre]
<cr1901_modern>
rjo: You broke resize with your changes by unconditionally setting the view during the resizeEvent; we don't know whether the slider will resize before the widget thanks to the layout, and by virtue of setting the stop/start positions in setView, the slider may (and does) set the sliders to the wrong values.
<sb0>
whitequark, how to start the win7-experimental vm_
<bb-m-labs>
I'll give a shout when the build finishes
<cr1901_modern>
rjo: We have to keep handle width around if we want resize to work properly. The handle size is not just an offset; the maximum usable slider width has to account for the handle width. If we don't account for the handle width, during a resize, then the real value that mapped to the slider_width - handle_width prior to resize will not map to new_slider_width - handle_width pixel. >>
<cr1901_modern>
In this case, the slider knowing only the old and new width (and not the order of resizing) is not sufficient to properly move the handles to the new locations. Qt's sliderValueFromPosition/sliderPositionFromValue functions have no concept of handle width.
<cr1901_modern>
The scale resize has to be proportional to whatever Qt thinks the slider's usable width is, so the slider can resize itself without any knowledge about whether the new transformation is in effect or not. The alternative is to make Qt think that the entire width of the slider is usable for mapping to slider space. Which... might not be a bad idea. Let me think it over.
<sb0>
whitequark, why are there artiq_flash and artiq_flash_1 steps?
<cr1901_modern>
Qt might not like that when drawing, but perhaps I could write a bit of custom code. This would also possibly satisfy sb0's request of slider handles disappearing gradually.
<GitHub105>
[artiq] jordens pushed 2 new commits to master: https://git.io/va8J7
<GitHub105>
artiq/master 1dc7263 Robert Jordens: scanwidget: changes as of 19a1d40 (closes #330)
<GitHub105>
artiq/master 2e73d6c Robert Jordens: scanwidget: changes as of 1f9ab9b (resize)
<rjo>
cr1901_modern: i think you probable have the root issue wrong. do you feel you are able to fix it now?
<rjo>
*probably
<cr1901_modern>
rjo: What makess you think that?
<rjo>
cr1901_modern: the resize behavior is fully controlled now.
<cr1901_modern>
rjo: It's possible for the parent scanwidget to resize before the axis and/or slider. I've seen it before.
<cr1901_modern>
i.e. printing out the order in which resize events are received
<rjo>
cr1901_modern: look at the code. axis and sliders don't get any resize events now.
FabM has quit [Remote host closed the connection]
<cr1901_modern>
rjo: ahhh, so they don't. That's interesting.
<cr1901_modern>
resizeEvent gets called twice during resize, and so now the order doesn't matter b/c the second time it's called, everything will be done.
<cr1901_modern>
rjo: That's clever
<cr1901_modern>
I kinda wish I thought of it
<cr1901_modern>
I *should've* thought of that
<GitHub179>
[misoc] enjoy-digital pushed 1 new commit to master: https://git.io/va8qL
<cr1901_modern>
rjo: The new code also no longer assumes order in the sense that even if the scanwidget itself receives its resizeEvent first (which it does at times, mainly during show()), the axis will repaint itself correctly because a paintEvent will be sent along with the resizeEvent.
<cr1901_modern>
The net effect of removing the handle width from resizeEvent is that the rightmost value of the widget increases/decreases slightly based on whether the widget is expanding/shrinking.
<rjo>
cr1901_modern: you should also do sliderPositionFromValue and its inverse yourself. you can derive that transformation directly from the proxy transformation, the width, and the groove offset and/or handleWidth/2.
<rjo>
i am confident that you can shave another 100 lines off that code without too many mental push ups.
<cr1901_modern>
cr1901_modern: I'm not prepared to implement that myself b/c I don't know if Qt does anything fancy based on subpixel rendering or other pixelmetrics based on the backend used.
<cr1901_modern>
rjo: ^
<cr1901_modern>
I'll check the source
<rjo>
it just pixels.
<rjo>
it's
<cr1901_modern>
And if me manually calculating the pixel to handle transform != what Qt thinks when drawing, the widget breaks. I don't know if it's "just pixels" and I feel it shouldn't be my code's responsibility to calculate that information. Qt knows how to draw the slider and what information is required to do so.
<cr1901_modern>
rjo: FWIW, I would think that Qt's implementation of pixel to handle value is naive and what you're asking me to do would work. I just don't want it breaking later b/c Qt decided to do something behind the scenes
<rjo>
that would break much earlier because you are already feeding it ad-hoc information that makes all kinds of assumptions.
<cr1901_modern>
That's another reason I feel that I shouldn't implement that then
<cr1901_modern>
But doesn't matter. I'll look into implementing if after I fix the handle width problem
<cr1901_modern>
rjo: Just an update: axis width != scanwidget width at any given moment. The old proxy event filter operated on the axis width. Thinking about the best course of action here.
<cr1901_modern>
We need the oldSize to calculate the new scale, so I think the best course of action here is just to do everything wrt the scanwidget width. But that will require the slider and axis to know about how they are positioned relative to their parent
<cr1901_modern>
This doesn't get any easier does it :P?
<cr1901_modern>
Might be time to finally just dump the scale and use left and right
<rjo>
cr1901_modern: when are they not equal? you always need to convert to pixels. you need a width (that's the same as a scale) to do that. if axis.width != scanwidget.width != slider.width then you have to find out either how to make them equal or how ensure this won't become a problem.
<cr1901_modern>
rjo: They appear to be not equal always, according to my print statements
<rjo>
if it's only transient, then that's fine.
<cr1901_modern>
transient?
<rjo>
e.g. while resize is in progress.
<cr1901_modern>
Let me check that
<cr1901_modern>
rjo: No it's not transient. They are never equal. Presumably due to the layout or something
<rjo>
the other thing that may be pretty easy is to merge all three classes into one. then you get rid of the layout and the axis and all sizes are always consistent.
<cr1901_modern>
Well, I could control painting manually like that. I'm guessing modularity isn't all that important for this widget?
<rjo>
the axis is really just a single paint method. and the proxy is just a layout that forwards events and does a transformation.
<rjo>
modularity is irrelevant. it's a single method.
<cr1901_modern>
rjo: Well, NOW it's irrelevant since code keeps getting moved around :P
<cr1901_modern>
rjo: probably for the best to merge them all, but let me check something
<cr1901_modern>
rjo: Let me know when you're done refactoring, so I can combine the classes. Probably the best bet at this point instead of passing data around.
<rjo>
cr1901_modern: i'm done
<cr1901_modern>
rjo: Can I bikeshed briefly? Feel free to decline
<cr1901_modern>
Nevermind, I'll just do it, and see what happens