search

rovsuite / monterey

Monterey is the computer GUI for rov-suite, an open source ROV control system.
21 stars 14 forks source link

LedIndicators do not scale to QQuickViewer size #40

Closed chriskonstad closed 11 years ago

chriskonstad commented 11 years ago

Please make the LedIndicators scalable. Right now, the text size and position does not change even if they need to. Otherwise, they work great!

small

large

Thanks!

QuantumCD commented 11 years ago

Okay, I can work on that better now that I know roughly the size they are going to be.

QuantumCD commented 11 years ago

Hey Chris. I was working on the scaling, and I was going to use a collapsing point to change certain properties so the text fits on one line with smaller font points and different heights/anchors. However, it didn't seem that the height property was being picked up in QML for some reason. I was using the debugger, and it was changing based on re-sizing the window, but the conditionals weren't triggering. Any ideas?

chriskonstad commented 11 years ago

Side note about the size: We should probably shrink the activity log QTextEdit and increase the height of the status light box.

chriskonstad commented 11 years ago

Which height property did the QML not see? The height of the QQuickView?

chriskonstad commented 11 years ago

By looking at the QML and the results, it looks like the base rectangle (id: background) is scaling properly. However, it looks like the text is not, as it is hard coded to pointSize 14. If you use Text.pixelSize instead of Text.pointSize, you can set the font size to the height that you assign the font element. The of the text element doesn't seem to affect the size of the font. Does that fix the issue? Also, which conditionals weren't triggering?

QuantumCD commented 11 years ago

Oh, no, I meant that the height property on background doesn't seem to be sending out a notify message like it should. This might be a bug with Qt itself, or just how the property is designed with the QQuickView. Basically, I was going to try and fake a "state" by using certain conditionals for the properties. e.g. 

height > collapseHeight ? parent.width * 0.2 : height

That would make the text field have the height of the background if that height were to fall below 35 (what I have the collapseHeight set to). In the debugger, the height value of background is changing accordingly, but these conditionals are not updating like they seem to be in my normal QML projects.

QuantumCD commented 11 years ago

Perhaps I could try a state, but if the value isn't notified, that will not work either. Also, the state and transition for the on/off value will have to be moved to some sort of animation instead. I could probably manage that part, but I am sort of at a loss as to why the height property isn't sending out an updated notifications when the SizeRootObjectToView is set as the resize mode.

chriskonstad commented 11 years ago

Oh! Okay. I see what you're doing now. It's not a binding issue (binding as opposed to assignment), is it? I'm relatively new to QML, so sorry if I'm not too helpful :)

We could add code to the C++ class LedIndicator to change the properties of elements in QML as a fallback option, too. If it is a notification issue, we could get around it using C++. Although that wouldn't be as clean of a solution....

QuantumCD commented 11 years ago

Yeah, that's what I was thinking. I'll look into a custom property then. Perhaps I could catch the resize event.

chriskonstad commented 11 years ago

Okay. Sounds good!

QuantumCD commented 11 years ago

Yeah... it turns out that this isn't going to be very pretty. I really wish that the height value was notifiable by default on the QQuickView. Time for hack-ish solution until I think of something more elegant. :)

chriskonstad commented 11 years ago

Okay. That works! I just changed the minimum height of the QGroupBox that holds those QQuickViews, so you might want to keep that in mind. You can now support a different range of heights that start off taller than before!

QuantumCD commented 11 years ago

Okay, I'll keep that in mind. What's really weird is that I did some testing, and the height seems to be updating. The conditional properties do not, however, which is what interests me. I'll keep working on it. I saw that you wanted to release Monterey 3.0 soon, and I didn't know you had a deadline!

QuantumCD commented 11 years ago

Oh wow! That was a silly error on my part. I needed to be using the background's height, not each components' height. I'm glad I finally figured that out.

QuantumCD commented 11 years ago

Okay, the bug seems to be fixed. Try it out and report back any changes you think should be made. This was only tested up to 1080p resolution, so I don't know what it would look like at higher resolutions (but that isn't very applicable until those 8K monitors become popular :P )

chriskonstad commented 11 years ago

Awesome! I think they look great, but maybe the indicator text could be slightly larger? It's easy enough to see the on/off text, but it could be hard to see the other text in bright light. What do you think? Haha! I think that 1080p will be our highest targeted resolution.... And sorry about not letting you know about the deadline! That's just an arbitrary goal that I set up. If Monterey's release date needs to be extended, it could be.