-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(scale): set tick drawing order #6185
base: master
Are you sure you want to change the base?
Conversation
Note: Not final revision but it seems to be working as expected. The existing render tests are passing. |
Thank you! Just please add a test for |
I took the code snippet of the previous pull request and made the scale bigger (in the original size the minor ticks look weird) We can notice that the sections doesn't exactly end in the major ticks, this is expected (unless somebody else implemented in) because when drawing arcs I wasn't able to find a way to increase the arc angle depending on the section tick widths. |
The error at
If it's a regression we should have a test to cover it in the future. |
I will check the sample code later today, I only copied and pasted it as is to run the examples. |
@kisvegabor Do you know a way to calculate the angle between the arc center and a given point? |
Try |
Your detailed description is very useful. I'll have a busy Friday and weekend but I will give it a go on Monday. |
Hi @kisvegabor, I have made more progress with the issue of the ticks/arcs misalignment. I don't know how to apply the 4 commit changes that @C47D made to my local repository (I tried git apply from the .patch files pulled from github, but it kept failing with errors; I'm sure someone with a lot more git experience would know how to fix the issue). However, I have managed to get the arcs and the ticks to align much better and without the current +1 requirement of the lv_scale_set_range. via changes to two lines (Note, the same changes would probably need to be applied to the straight line scale). Using:
Using:
The output is: The changes are:
I have not looked into the arc drawing yet, just the tick part so far (I'm still very slow with c), but its so close that I'm quite buzzing with excitement at having got this far. Its interesting that the alignment of the arc seems to be off in exactly the same place and amount for the blue and green. Also interesting is that the major tick at "20" isn't green, as it should be because 6*64=384. That the two screen caps now match much more closely with different range scales is hopefully a positive step. I did find that to get the first major green tick to go grey requires a value of 324 and to get the grey major tick to turn green requires 388. This suggests the issue is possibly similar to the needle issue (accuracy) or there is maybe some offsetting done. If I get some time tomorrow afternoon (I'm on UK time) after dropping the granddaughter off I'll have a look into section colour code and also the arc code. |
I have now resolved the issue of the ticks which are now drawing correctly for all examples and changing colour at the correct points. The reason it didn't before is because the calls to lv_map() were subtracting 1 which obviously threw the calculations out. The changes have been made to both the straight edge scale calculations and the arc edge scale calculations so that they no longer require +1's for the ticks and/or the ranges. A tick count of 10 means 0-10 ticks. The changes however required a change to the examples to make them work. Obviously this was because they were still adding a +1 to the total tick count. This is obviously a breaking API change so I don't know how you would want to handle that, or if such a change is a serious NO. As I wasn't working on the changes with @C47D changes applied I've created a diff of the changes. The coloured sweeping arcs are still off slightly but I felt that maybe the two issues should be dealt with in two different patches. I've attached the diff patch file (in a zip as github doesn't seem to like attaching .patch files or I've not created a valid patch file!). I hope this helps and is acceptable. |
Hi @i400s thanks for the investigation, I had a long weekend so I'm just reading your findings. So the main issue seems to be that the tick count considers the initial tick (tick 0 let's call it) into the tick count? If so I think that's the expected behavior, is that correct @kisvegabor ? If we change it it might break all of the usages of the scale out there I think. |
Hi @C47D. Due to your post (the edited version) I've had another look at the code and I think my changes are completely superfluous to requirements. If on my repository I performed a The alignment/tick colour issues I was having was down to talking my original meter code and just transplanting it with the scale code and not completely understanding the link between ticks and range. The +1 requirement of the ticks (which then gets subtracted in the lv_scale code) lead me to think that the range also needed to some how be off by some variation of +1 or (number of ticks * multiplier) +1 which then obviously meant the alignments were off which I was then trying to correct by tweaking the sections instead of fixing the error with the base scale range. My changes, it seems, are completely redundant which is somewhat embarrassing (although it has allowed me to get a better grip on the code). Which means there is no need to break the API. I guessing my misunderstanding was probably quite unique due to the way I wanted my scale to have sub-ticks for fractions of degrees (so I could move the needle with greater precision than the minor ticks). |
Wow, what an interesting outcome! If the current method is working as it is, we at least need t describe it in the docs (maybe in the functions' API comments too) and add a dedicated example. As you understand much better than me what is happening, could you add these? |
Don't worry, I felt the same when coming back to the scale code, it's been a while since I first did it. I'll check the code to find the divisions and try to make it less prone to rounding errors.
Do you have any suggestions to improve the docs @kisvegabor ? |
I think we should add 2 things:
|
I've created an example you may find useful. Eventually my intention is to use it, and an ESP32C6, to control my central heating via a simple relay replacing a rather old timer/temp switch. Actual result on a 2.9" device in dark mode (currently on a breadboard): The LED switches on/off as the needle crosses a temperature. It may need some cleaning up in that I prefix statics with an s_ But it does use calculations to work out the scales/ranges and the interaction between them (including on the temperature bands and the led switching). Zip file containing 1 header, and 1 c file: |
Hi, I was in a rather busy week at work, will have some free time next Wednesday to take a look at this 😸 |
We need some feedback on this pull request. Now we mark this as "stale" because there was no activity here for 14 days. Remove the "stale" label or comment else this will be closed in 7 days. |
Not stale, working on it |
Hi @kisvegabor , Is there any documentation on how Also, since this request was originated to be able to customize the drawing order, can we merge it and improve the |
It's exactly the same as the fairly well known Arduino |
Sure, just please add a few lines about this new feature in the docs. |
Done, I've also added a couple of reference images. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks good, just please remove the mouse cursor from the image in the docs 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the new function scale_calculate_main_compensation
is based on scale_draw_indicator
. The only difference is it stores compensation values instead of drawing lines. It's ~170 replicated lines.
I would be happy to see the logic modularized into one function that is used to compute the info, and the caller can use that info to draw the lines, or store it. Currently scale_calculate_main_compensation
and scale_draw_indicator
must have the same contents. I could see them getting out of sync in later changes.
What do you think?
Hi, Yes, I'll work on cleaning up the code, I think the lowest hanging fruit is moving the styles initialization outside from the drawing functions to a common place, what do you think? |
Hi @liamHowatt , I've worked in the indicator function, most of the function was duplicated, I'll work on the compensation calculation function in a similar way. |
Description of the feature or fix
Be able to change the drawing order of the tick widget
Notes
lv_conf_template.h
run lv_conf_internal_gen.py and update Kconfig.scripts/code-format.py
(astyle version v3.4.12 needs to be installed) and follow the Code Conventions.