Conversation
Use shared_ptr to take single frames from the DynamicDiscretisedDensity.
|
If there is interest, I have an additional class that can calculate simple types of motion (rotation for now) and refresh the shapes in each time frame. |
In the frame comparison we need to subtract 1 to be consistent
robbietuk
left a comment
robbietuk
left a comment
There was a problem hiding this comment.
I like it. Just a few comments or concerns.
examples/ROOT_files/ROOT_STIR_consistency/SourceFiles/.generate_image1.par.swp is a binary file, is it needed?
| get_output_sptr() | ||
| get_output_sptr(unsigned int frame) | ||
| { | ||
| return out_density_ptr; | ||
| return out_density_ptr->get_density_sptr(frame); | ||
| } | ||
|
|
||
| shared_ptr<DynamicDiscretisedDensity> | ||
| GenerateImage:: | ||
| get_all_outputs_sptr() | ||
| { | ||
| return out_density_ptr; | ||
| } |
There was a problem hiding this comment.
Why not keep get_output_sptr() with no arg and add get_output_sptr(unsigned int frame) method? Maybe this is clearer?
There was a problem hiding this comment.
it would be, or actually we wouldn't really need get_output_sptr(frame) really. However, sadly I cannot see a way to preserve backwards compatibility.
|
|
||
| //! Returns the discretised density with computed shapes. | ||
| shared_ptr<DiscretisedDensity<3, float>> get_output_sptr(); | ||
| shared_ptr<DiscretisedDensity<3, float>> get_output_sptr(unsigned int frame = 1); |
There was a problem hiding this comment.
It seems like this default might cause problems later?
There was a problem hiding this comment.
Sorry, bad comment. I think I meant it doesn't preserve backways compatability.
It would be of interest, but it'd have to work with the motion stuff in |
KrisThielemans
left a comment
KrisThielemans
left a comment
There was a problem hiding this comment.
Interesting and small. I'm not so sure I like adding frames to the Shape3D class. It seems frames don't have too much to do with a shape.
Maybe there's a way to change the value keyword into a TAC? Might be less obvious, but would be quite nice...
| out_density_ptr.reset(new DynamicDiscretisedDensity(exam_info_sptr->get_time_frame_definitions(), | ||
| rel_start_time, | ||
| scn, | ||
| tmpl_image) ); | ||
|
|
There was a problem hiding this comment.
I'm astonished that we don't have a constructor for DynamicDiscretisedDensity that takes an ExamInfo. That would be far better. Maybe add it in this PR?
| const singleDiscDensT & | ||
| get_density(const unsigned int frame_num) const ; | ||
|
|
||
| shared_ptr<DiscretisedDensity<3, float> > |
There was a problem hiding this comment.
I think would need to return shared_ptr<const singleDiscDensT> : correct template but also prevents modifying the underlying object.
|
Let's do that for STIR 6.0, i.e. after release of 5.2, as then we can break backwards compatibility as much as we like! Anyone any ideas on adding a TAC to shapes? Ideally would include integration of the TAC for a time-frame. Possibly we can re-use some of the |
3 participants
Added the simple ability to generate dynamic images based on a .fdef file.
The modifications are simple and backward compatibility is maintained.