Particle system - Part 1 #516

caguero · 2020-12-23T21:28:35Z

The goal is to integrate a particle system that can use the particle emitters available in Ignition Rendering.

This patch focuses on the creation of a new ParticleEmitter component.

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

codecov · 2020-12-23T22:27:49Z

Codecov Report

Merging #516 (a830b44) into ign-gazebo4 (302f5ed) will increase coverage by 0.23%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           ign-gazebo4     #516      +/-   ##
===============================================
+ Coverage        77.37%   77.61%   +0.23%     
===============================================
  Files              217      213       -4     
  Lines            12217    11624     -593     
===============================================
- Hits              9453     9022     -431     
+ Misses            2764     2602     -162

Impacted Files	Coverage Δ
src/systems/user_commands/UserCommands.cc	`77.86% <0.00%> (-1.81%)`	⬇️
src/EntityComponentManager.cc	`84.49% <0.00%> (-1.54%)`	⬇️
src/systems/diff_drive/DiffDrive.cc	`84.10% <0.00%> (-1.00%)`	⬇️
src/network/NetworkManagerSecondary.cc	`80.64% <0.00%> (-0.90%)`	⬇️
src/systems/velocity_control/VelocityControl.cc	`83.01% <0.00%> (-0.62%)`	⬇️
src/gui/PathManager.cc	`88.88% <0.00%> (-0.59%)`	⬇️
src/systems/logical_camera/LogicalCamera.cc	`77.77% <0.00%> (-0.31%)`	⬇️
src/Util.cc	`95.15% <0.00%> (-0.31%)`	⬇️
src/Conversions.cc	`81.63% <0.00%> (-0.26%)`	⬇️
.../systems/triggered_publisher/TriggeredPublisher.cc	`81.97% <0.00%> (-0.16%)`	⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44a6713...04721d3. Read the comment docs.

include/ignition/gazebo/components/ParticleEmitter.hh

…ter. Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

chapulina

Thanks for iterating!

chapulina · 2021-01-30T02:40:40Z

@caguero , would you prefer merging these PRs as they are approved, or should we wait until all the functionality is working and approved before merging all PRs? Note that this is targeting a stable release branch, and whatever is merged in an intermediary state may be released at any point.

caguero · 2021-01-30T10:33:05Z

@caguero , would you prefer merging these PRs as they are approved, or should we wait until all the functionality is working and approved before merging all tutorials? Note that this is targeting a stable release branch, and whatever is merged in an intermediary state may be released at any point.

We can review part1, part2 and part3 and merge them all at once when we're happy with it. These three PRs offer the basic integration. Part4 (coming up) is more independent.

chapulina · 2021-02-01T18:22:39Z

We can review part1, part2 and part3 and merge them all at once when we're happy with it.

Got it. I converted this to draft so that we don't merge it by mistake.

adlarkin · 2021-02-08T23:07:30Z

I'm noticing the following error in CI:

  /github/workspace/include/ignition/gazebo/components/ParticleEmitter.hh:20:10: fatal error: ignition/msgs/particle_emitter.pb.h: No such file or directory
   #include <ignition/msgs/particle_emitter.pb.h>

I believe that we need to make an ign-msgs6 release. cc @chapulina @iche033 let me know if you'd like for me to make an ign-msgs6 release

chapulina · 2021-02-09T00:57:42Z

ign-msgs6 release.

I'm fine with a release, my only request is that we make sure the current particle message has all we need, because this will go into a stable release, and we won't be able to tweak any fields afterwards. So maybe it's best to wait until all parts are out beforehand. But if @iche033 thinks the implementation is already settled, then go for it.

adlarkin · 2021-02-09T01:01:52Z

my only request is that we make sure the current particle message has all we need, because this will go into a stable release, and we won't be able to tweak any fields afterwards.

Good point. I am still reviewing the other particle system PRs, so I will make a release once we know all of the other PRs are good to go 👍

iche033 · 2021-02-09T01:50:43Z

yeah let's hold off until we have everything reviewed and ready to go. There's still some work left that need to be on on particles this week.

Signed-off-by: Carlos Agüero <caguero@osrfoundation.org> Co-authored-by: Ashton Larkin <ashton@openrobotics.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>

adlarkin · 2021-02-18T01:57:41Z

@iche033 there was a merge conflict in 5885e45 that I resolved. It looks like #482 added some code to RenderUtil::UpdateECM to update lights, so I kept that addition along with the additions we made to RenderUtil::UpdateECM.

adlarkin

A few minor comments that would be nice to address before merging, but I'm fine with merging this PR as-is.

src/systems/particle_emitter/ParticleEmitter.cc

test/integration/particle_emitter.cc

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Signed-off-by: Nate Koenig <nate@openrobotics.org>

caguero added 2 commits December 23, 2020 22:18

New ParticleEmitter component.

4a9aca8

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

Remove attribute name.

272d39b

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

caguero requested a review from chapulina as a code owner December 23, 2020 21:28

github-actions bot added the 🔮 dome Ignition Dome label Dec 23, 2020

Merge branch 'ign-gazebo4' into particle_system

fb0c084

chapulina added the rendering Involves Ignition Rendering label Dec 23, 2020

chapulina reviewed Dec 29, 2020

View reviewed changes

include/ignition/gazebo/components/ParticleEmitter.hh Outdated Show resolved Hide resolved

caguero requested a review from iche033 January 4, 2021 16:20

caguero and others added 3 commits January 7, 2021 17:37

Update particle emitter component to use ignition::msgs::ParticleEmit…

698bb1e

…ter. Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

Merge

24261cf

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

Merge branch 'ign-gazebo4' into particle_system

f5538d5

chapulina mentioned this pull request Jan 13, 2021

Support particle effects #558

Closed

caguero mentioned this pull request Jan 15, 2021

Particle system - Part2 #562

Merged

caguero and others added 2 commits January 29, 2021 16:53

Merge branch 'ign-gazebo4' into particle_system

0a26d4e

Simplify particle emitter component.

4cbc9dd

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>

chapulina added the needs upstream release Blocked by a release of an upstream library label Jan 29, 2021

chapulina approved these changes Jan 29, 2021

View reviewed changes

Merge branch 'ign-gazebo4' into particle_system

9496d8f

chapulina marked this pull request as draft February 1, 2021 18:21

Merge branch 'ign-gazebo4' into particle_system

a02824c

Merge branch 'ign-gazebo4' into particle_system

2ed1013

Merge branch 'ign-gazebo4' into particle_system

8c13dce

iche033 marked this pull request as ready for review February 17, 2021 05:01

nkoenig mentioned this pull request Feb 17, 2021

6.3.0 prep gazebosim/gz-msgs#136

Merged

7 tasks

adlarkin mentioned this pull request Feb 17, 2021

4.5.0 gazebosim/gz-rendering#254

Merged

7 tasks

Particle system - Part2 (#562)

e77e376

Signed-off-by: Carlos Agüero <caguero@osrfoundation.org> Co-authored-by: Ashton Larkin <ashton@openrobotics.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>

adlarkin force-pushed the particle_system branch from fdb02c2 to e77e376 Compare February 18, 2021 01:51

Merge branch 'ign-gazebo4' into particle_system

5885e45

adlarkin approved these changes Feb 18, 2021

View reviewed changes

src/systems/particle_emitter/ParticleEmitter.cc Show resolved Hide resolved

test/integration/particle_emitter.cc Show resolved Hide resolved

test/integration/particle_emitter.cc Show resolved Hide resolved

test/integration/particle_emitter.cc Outdated Show resolved Hide resolved

iche033 and others added 4 commits February 17, 2021 18:28

bump msgs version to 6.3.0

7f122de

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Remove extra main

8a1db3d

Signed-off-by: Nate Koenig <nate@openrobotics.org>

Added missing <set>

494a11e

Signed-off-by: Nate Koenig <nate@openrobotics.org>

4.5.0

04721d3

Signed-off-by: Nate Koenig <nate@openrobotics.org>

nkoenig merged commit deddb6f into ign-gazebo4 Feb 18, 2021

nkoenig deleted the particle_system branch February 18, 2021 05:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Particle system - Part 1 #516

Particle system - Part 1 #516

caguero commented Dec 23, 2020 •

edited

Loading

codecov bot commented Dec 23, 2020 •

edited

Loading

chapulina left a comment

chapulina commented Jan 30, 2021 •

edited

Loading

caguero commented Jan 30, 2021

chapulina commented Feb 1, 2021

adlarkin commented Feb 8, 2021 •

edited

Loading

chapulina commented Feb 9, 2021 •

edited

Loading

adlarkin commented Feb 9, 2021

iche033 commented Feb 9, 2021

adlarkin commented Feb 18, 2021

adlarkin left a comment

Particle system - Part 1 #516

Particle system - Part 1 #516

Conversation

caguero commented Dec 23, 2020 • edited Loading

codecov bot commented Dec 23, 2020 • edited Loading

Codecov Report

chapulina left a comment

Choose a reason for hiding this comment

chapulina commented Jan 30, 2021 • edited Loading

caguero commented Jan 30, 2021

chapulina commented Feb 1, 2021

adlarkin commented Feb 8, 2021 • edited Loading

chapulina commented Feb 9, 2021 • edited Loading

adlarkin commented Feb 9, 2021

iche033 commented Feb 9, 2021

adlarkin commented Feb 18, 2021

adlarkin left a comment

Choose a reason for hiding this comment

caguero commented Dec 23, 2020 •

edited

Loading

codecov bot commented Dec 23, 2020 •

edited

Loading

chapulina commented Jan 30, 2021 •

edited

Loading

adlarkin commented Feb 8, 2021 •

edited

Loading

chapulina commented Feb 9, 2021 •

edited

Loading