-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Particle system and segment/effect blending #4612
base: main
Are you sure you want to change the base?
Conversation
- allow incrementing/decrementing as specified in API
- additional debug info
- additional debug info
- credit @DedeHai - prevents flickering on C3 when accessing FS
- credit @DedeHai - prevents flickering on C3 when accessing FS
The Unit of Measurement ("tempScale") of the MQTT message is set for each published measurement - but not for the homeassistant discovery. Thus, openhab (and mabye other systems?) don't recognize the value as "Number" - so it uses String instead. This is somehow annoying when trying to configure the sensor channel to be linked to an existing item in OpenHAB. (Items that are created automatically or with "Add point to model" can be configured in a way that the String is transformed to Number or Number:Temperature, but existing Items cannot be linked). When a "Unit of Measurement" is set in HomeAssistant discovery, the HA binding of OpenHAB notices that the MQTT String is a number and automatically converts it.
- credit @DedeHai - prevents flickering on C3 when accessing FS
is it possible to pull in latest 0.16 to see the actual changes? very hard to see what changed and how to update the PS as every file has many changes to 0.15... |
most likely (with conflicts to resolve) but I'm not willing to invest time at this stage. unfortunately. As I see it the biggest problem lies in memory management and segment handling within PS. I may be wrong, though. |
Ok, got it. As mentioned before, most memory management can be dropped. So what exactly should I do? just remove segment dependencies where possible? |
Try simplifying environment management by removing anything that deals with outside environment, i.e. non-PS stuff like segment, transitions or effect blending and use setPixelColor/getPixelColor instead of memcpy or similar local buffer management. As I understand (from examining PS code) you needed to implement your own memory management to allow class instantiation and allocation of particle data. You also moved some of that handling into strip.service() call (to eventually release unused memory). We may need to find a better approach for that by letting segment to inform effect/PS when certain events happen (start, stop, restart, resize, etc.) Work in this branch (pull it into your repo and work on it, you'll be able to push) as there is no need to modify upstream at this point. |
I'll try to break down the current state and the ideas behind it, so maybe we can work out a better way to do it:
The way I see it, sharing the particle-buffer between FX of the same segment is a much better approach than using up even more RAM by not sharing the buffer. Limiting the buffer to a smaller size is possible but then the limit is again about 32x32, beyond that the FX will not look good.
|
Ok. Here are my thoughts:
Segments do not need ID internally, it would be nightmare to handle those as segments can be deleted in the middle of stack. And it is perfectly ok if there are gaps in vector. As mentioned elsewhere (perhaps to @willmmiles ) we only need to tell effect function that it:
The function that handles (should handle) these notifications is Regarding sharing of buffers: Nothing prohibits you from doing that. However take into account that user may overlay several segments (probably partially) and each segment may or may not include effect from PS. How will those particles interact will be interesting to see. To be clear, if you cannot make PS work with segment layering (which includes better version of effect transitions; due to memory or other constrains) it will just mean that layering will not get into upstream as I do not want to force or remove anything. The code here is provided with intent that you can check what does it mean for PS. |
not really, the PS is part of that effect, just like a for-loop filling an array is in a normal effect, but more elaborate and as a class is how I see it
PS does not generate effects, it just provides methods to do so but that is just semantics. The pointers need to be updated just like any other effect does if data pointer moves, it just does that in a function.
alright, can do (and it's what it was at one point)
I think extending 'allocateData()' may be a good way. A stop notifcation would mean after a stop, the FX needs to run one last time to let it clean up, not sure that is possible with current handling. If start and stop is directed to the mem-manager, it could also work (and keep PS memory handling out of segment functionality). I need to think about it.
keeping a list of ID's is not what I would call a nightmare. Give each segment a new variable (`uID'), when creating a segment, check for used IDs in a loop, choose a free number. If the segment moves in the vector, the uID stays, making it easier to link memory to a segment.
Particles do not interact really, just their position is kept when transferring to another FX, but removing that is as easy as setting transferred buffer to zero (which is already in place, but it deliberately keeps the positions). Like overlayed segments have their own pixel-buffer, it also has its own particle buffer. That is already how it is now, particle buffer is only shared during transitions so there is no extra memory needed during the transition.
I don't see any issue, it does work now, it did work before (without shared buffers) and the layering should make it easier. I just need to understand how the segment handling changed. |
…uction from panels
- (re)moved gamma to the last step in LED output - reconstructed all cpt-city palettes to be uncorrected - added Python script to download and convert cpt-city palettes - misc: removed panels counter - FX: added palette option to Flow Stripe and Drift Rose
- made a few methods protected - added safeguards for segment changing - increased vertical 2D dimensions to 16 bit
wled00/FX.cpp
Outdated
@@ -6109,13 +6107,13 @@ uint16_t mode_2Ddriftrose(void) { | |||
float angle = radians(i * 10); | |||
uint32_t x = (CX + (sin_t(angle) * (beatsin8_t(i, 0, L*2)-L))) * 255.f; | |||
uint32_t y = (CY + (cos_t(angle) * (beatsin8_t(i, 0, L*2)-L))) * 255.f; | |||
SEGMENT.wu_pixel(x, y, CHSV(i * 10, 255, 255)); | |||
SEGMENT.wu_pixel(x, y, SEGMENT.palette==0 ? (uint32_t)CRGBW(CHSV(i * 10, 255, 255)) : ColorFromPalette(SEGPALETTE, map(i, 1,37, 0,255), 255, LINEARBLEND)); |
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.
question:
above you used colorwheel, here you keep the CHSV variant. is colorwheel something that should be updated? it does basically HSV to RGB but in a crude way... could make use of HSV2RGB function instead
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.
Good catch. Will fix.
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.
oh, I though you did that intententionally, hence the question. So should colorwheel be updated to use CHSV -> CRGBW i.e. HSV2RGB_rainbow() so it uses a better/more accurate color conversion when not using palette or is there a specific reason it is the way it is (other than legacy)?
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.
Anything using CHSV with full saturation and value should be replaced by color_wheel as that allows use of palettes.
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.
yes it should. colorwheel should then be maybe an inline function to not reduce speed (I can check if it makes a big difference) and call hsv2rgb_rainbow()
…uction from panels
- (re)moved gamma to the last step in LED output - reconstructed all cpt-city palettes to be uncorrected - added Python script to download and convert cpt-city palettes - misc: removed panels counter - FX: added palette option to Flow Stripe and Drift Rose
- made a few methods protected - added safeguards for segment changing - increased vertical 2D dimensions to 16 bit
- undo strip.trigger() change
- reduce 'useMainSegmentOnly' checks - add clarification - use strip.trigger() in useMainSegmentOlny
- reordered class members for better alignment - moved transition progress into Transition class - made more functions protected - improved transition handling - tweaks in segment blending
Draft PR for @DedeHai to start working on integration of particle system into unified segment and effect blending using layering technique.
Based on 0.15 code.