Conversation
Member
|
Can you provide the actual error from the compiler too? I'll have to check this one closely, as the |
Contributor
Author
|
@wjwwood Here you go.
Absolutely -- I'm definitely not sure these operations are equivalent. |
Contributor
|
I believe the main point of the funciton If we replace this I would suggest to replace the whole function with |
Member
|
@anhosi I was leaning to something similar, though I don't know how the performance will be impacted. I'm going to test and, if working, take this pr and then make a follow up issue to investigate cleaning this particular segment of code up. |
60f33e5 to
1327bf4
Compare
1327bf4 to
3fa9b04
Compare
Member
|
Ok, I tested this locally, and it does seem to work. To test it I ported this To test this I opened a few shells with the ros2 workspace sourced and then I:
CI:
|
anhosi
added a commit
to bosch-io/rviz
that referenced
this pull request
Dec 19, 2017
- was made obsolete by pr ros2#136 which removed the memcopy
wjwwood
pushed a commit
that referenced
this pull request
Jan 3, 2018
- was made obsolete by pr #136 which removed the memcopy
wjwwood
added a commit
that referenced
this pull request
Feb 8, 2018
* add dependency between rviz2 and rviz_default_plugins (#149) * Move pointcloud displays to rviz_default_plugins (#153) * Move pointcloud displays to rviz_default_plugins * Move pointcloud tests to rviz_default_plugins * Adjust namespaces for pointcloud display files * Fix namespaces and import for pointcloud tests * Rename transformers folder * Fix linting errors * Move pointcloud files between CMakeLists, remove display from display_factory * Add pointcloud displays to plugins_description.xml * fix includes * add missing test dependencies * uncrustify * Revert "uncrustify" This reverts commit 03ed305. * disable uncrustify on long include statements * Fixed missing break in switch-case (#158) * Fall-through is not intended: listAppendNew sets the type to List while the call to setValue after the fall-through resets the type to Value. * Remove now obsolete function (#163) - was made obsolete by pr #136 which removed the memcopy * Reenable and fix config loading (#167) * Switch to assimp version 4.1.0 (#169) This should fix the dynimcally linked assimp build on Windows. * Feature/grid display refactoring (#165) * Move billboard_line to rviz2 for use in grid * Restore billboard functionality in Grid * Uncrustify + cpplint * Add tests for billboard_line - need to expose getChains for testing - one test documents faulty behaviour of Ogre in test * Refactoring of billboard_line - extract functions - fix functionality for huge numbers of elements per chain - add TODO comment to use internal Ogre functionality when fixed * Add tests for grid - need to expose functions for testing * Refactoring of grid * Propose additional newLine() possible in BillboardLine for ease of use - this gets rid of awkward "if(first element) don't add a new line" - simplifies usage - allows further refactoring of grid (for instance) * Minor refactoring of GridDisplay * Disable tests for now * Add functional header and std:: for "bind" * Introduce ROS interface abstraction to improve testability (#156) * Introduce interface for ros abstraction - move all public free function of ros_integration namespace to interface - add test for correct use of ros shutdown in VisualizerApp * Document recommendation for testable code * Move free functions from rclcpp_node_storage into RosAbstraction class * Rename RosAbstraction to RosClientAbstraction * Introduce RosNodeAbstraction for the node-centric part of the ros interface - also replaces the free function in get_topic_names_and_types * Cleanup: remove default arguments for virtual function * Refactor towards proper usage of RosNodeAbstraction * nitpick: alphabetical ordering * Comment only non interface classes (avoid duplicate documentation) * Cleanup: use consistent variable naming * Extract storage functionality from RosNodeAbstraction * Refactor RosNodeAbstraction towards a replacement of rclcpp::Node - a RosNodeAbstraction shall encapsulate a rclcpp::Node in the future and thus keeps track of its name (this simplifies a few functions) - for testing the rest of rviz the public api from the ros_integration namespace should be mocked - the optional storage parameter of RosNodeAbstraction is intended for unit testing of RosNodeAbstraction only and should not be needed otherwise * Fix gmock dependency * Migrate ImageDisplay (#164) * Reintroduce RosTopicProperty to MessageFilterDisplays - we can't correctly filter for message types. Added ToDo - Allow empty names in RosTopicProperty for now Refactor MessageFilterDisplay to correctly handle exceptions * Move ros_image_texture to default_plugins - Does mostly Ogre things, so definitely not rviz_common - Fix linters in RosImageTexture * Show image display in GUI and render default image Fix linters in rviz_rendering Improve naming of RenderWindow::setupSceneAfterInit() Move RenderPanel header to include folder * Let ImageDisplay extend MessageFilterDisplay to receive ROS image messages * Fix uncrustify and cpplint for ImageDisplay * Rename MessageFilterDisplay to RosTopicDisplay Remove the message filter parts currently commented out. Add comments regarding the missing message filter display * Add reliability profile to RosTopicDisplay * Introduce QueueSizeProperty * Put node from Display into _RosTopicDisplay * Initial move of ImageDisplay * Make ImageDisplay compile (except for missing image_display_base) * Reintroduce QueueSizeProperty to displays - Both ImageDisplay and PointCloud had the property previously * Offer generic way to change QoS objects * Expose ogre testing enviroment header in rviz_rendering * Add tests for ROSImageTexture * Use override instead of virtual where applicable * Extract scene setup lambda into separate method * Fix typo * Adapt interface of normalize and add tests * Interface abstraction for ROSImageTexture, replace raw pointers * Add onInitialize(string topic_name) to RosTopicDisplay This onInitialize automatically sets the default topic name to listen to * Move Image display to rviz_default_plugins * Add first tests for image display * Refactor ros_image_texture * Remove misleading onInitialize method from RosTopicDisplay * Disable tests for now * Implement review comments * Remove unnecessary check * fix memory leak (remake) (#173) * fix memory leak Signed-off-by: Chris Ye <chris.ye@intel.com> * remove vestigial example scene * use unique_ptr instead of shared_ptr and add comment * cpplint * Fix Windows build (#175) * wip * Fix visibility in rviz_common - needs to export some more symbols from rviz_common - small uncrustify changes * Fix stl_loader * Use dynamically linked OgrePlugins Don't link against Plugins anymore (loaded dynamically) Set ogre_plugin_dir to load dynamically Release and Debug need different behaviour * Fix yaml-cpp dynamic linking see jbeder/yaml-cpp#332 * Install rviz_rendering dlls * Reenable Windows tests * Move test folder to correct place - this is now consistent with rviz_common and rviz_default_plugins - fixup location of ogre_testing_environment - ogre_testing_environment needs to be exported * Fix linking against Ogre * Add zlib to be build on Windows * Change targets to unknown instead of shared * Fix billboard_line tests for Windows - need to expose symbols of billboard line for Windows - copy constructor of billboard line is not generated on Windows * Use rviz_common::UniformStringStream instead of std::stringstream * Fix linking of tests on Windows - just like rviz2.exe, tests need dll directories in %PATH% - add local install to environment variables for testing - in isolated builds, ogre resources cannot be found otherwise - enable tests in rviz_default_plugins on Windows but disable display * Fix grid_test * Linking to exported template function is problematic on Windows - this does not work with RVIZ_DEFAULT_PLUGINS_PUBLIC - delete tests for templated function - hide templated function again * Fix image_display tests * Introduce visibility control in rviz_default_plugins * run windeployqt on rviz2 binary to make it runnable after installation * extend PATH on Windows with location of libraries for vendor packages * Add missing visibility_control.hpp and fix license * log build output of dependencies to file * Fix Windows warnings * delete visibility from rviz_default_plugins * Disable deprecation warnings in Ogre * Fix dllexport warning in ros abstraction * fix linking warnings in rviz_rendering * fix linking errors * Fix warning in rviz_rendering * Reintroduce destructor to fix test * TF Display migration (#182) * Move tf_display * Make tf_display compile * Dissect tf_display into several classes * C++14 style adjustments of rviz_default_plugin tf classes - frame_selection_handler - frame_info - tf_display * Fix infinite loop when adding a display by topic * Rename tf_display folder to just tf * Expose functions for testability - BoundingRadius because it might need refactoring - getRenderOperation to allow updating Add tests to movable_text - Some tests already show bugs * Refactor movable_text - delete duplicate code in _setupGeometry() Further refactoring * Fix a couple of bugs - Line spacing could be set but did not have an effect - Using HorizontalAlignment::H_CENTER and text with spaces did not work correctly - Using VerticalAlignment::V_ABOVE did not work correctly * Extract more functions and refactoring * Set reasonable default value for spacing from the start * Refactoring of file style - Remove unnecessary variables - Delete non-virtual override that is protected - Remove duplicate method to getBoundingBox - Remove using namespace Ogre * Refactoring of tests - Expose update functionality instead of getRenderOp for testing - Change tests to approximate compare * Split methods in tf_display * Move some methods from tf_display to frame_info * Add tf_display to plugins_description * Change inheritance of MovableText to avoid visibility problems * Update Ogre to 1.10.11 (#181) * Use Ogre 1.10.11 * Fix deprecation warnings * Fix billboard_line - Ogre::BillboardLine is fixed, so we can remove manual counting - newLine() really finishes the line, thus rename to finishLine() * Fix cmake on Linux not appending _d any more * Migrate camera display (#183) * Initial move * Move display_group_visibility_properties * Make camera display compile * Add camera display to default plugins * Quickfix Segfault * First try * First working camera display * Shows the main scene from a different perspective * Misses visibility bits * Ignores caminfo status * Adds queue size property, visibility masks, listener removal on destruct Also includes a dummy caminfo for testing * Does not crash on startup, without visibility property * Reenable visibility properties * Add subscriber for CameraInfo messages * Fix linter issues * Reenable exact time mode * Implementing review comments * Extract smaller methods from large methods, cleanup Cleanup such as * correct float conversion * use lambda instead of bind * remove old comments * More method extraction and cleanup * Clean up and refactor display visibility properties * Split onInitialize method, remove obsolete comments * Fix visibility of BitAllocator * Fix Windows visibility * Default initialize forced_hidden (#187) * point to the source and bugtracker used in ros2 rather than the upstream one (#190) * Fix pointcloud2 display not accepting valid points (#189) * Fix rviz crash when removing a display (#191) The crash occurred when adding a camera display and then deleting any display that was created before adding the camera display. * print message to stdout instead of stderr (#193) * fix possible null pointer is dereferenced (#178) Signed-off-by: Chris Ye <chris.ye@intel.com> * Migrate polygon display (#194) * Initial move * Add to plugins_description.xml * Make display compile * Uncrustify * Change from uint32_t to size_t to silence warning on Windows. * Minor refactoring. * Enable alpha blending for the Polygon Display. * Let Ogre delete its render windows (#195) * Fix warnings from pluginlib (#196) * Update rviz2 installation to run it using `ros2 run` (#198) * disable RobotModel until it can be refactored to not use hard coded paths * Disable anti aliasing on Windows (#199) This fixes rendering issues on Windows when opening two or more render windows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GCC 8 has some rather strong feelings about the
memcpycall seen here. This changes it to a for loop so that it uses the=operator to copy data.