Skip to content
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

Issues with EVE7 #1

Open
7 of 26 tasks
rete opened this issue Nov 11, 2019 · 17 comments
Open
7 of 26 tasks

Issues with EVE7 #1

rete opened this issue Nov 11, 2019 · 17 comments

Comments

@rete
Copy link
Owner

rete commented Nov 11, 2019

This issue page is meant for keeping track of all issues found in the ROOT experimental eve7 component. This issue will remain open until a "stable enough" version of eve7 will be provided.


1. Bugs

General

Left menu

Event scene


2. Feature requests

General

  • No interaction with geometry scene at all. A menu like the settings in the geom viewer would be nice
    • Will be added soon

Left menu

  • After opening one event data collection and clicked on edit, a menu shows up down to edit the collection attributes (Rndr self, children, color, ...). Would be nice to add a close button to this menu
  • Left menu context menu:
    • Render only selected
    • Render all except selected
    • Render all
    • Render none
  • Feature request: left-click on element, highlight element on scene if rendered
  • In general the font size is too big. The mount of information shown in the left menu is not enough. The standard TEveManager had a nice vertical text spacing.
    • Dirty hack: CTRL+ - to reduce the font size
  • The sub EveElement indent is not enough. For example, on the picture below, the indent before Particle 0 in the sub list of PandoraPFOs is not enough. It's a bit hard to see which element belongs to which parent element.
  • Add check box instead of "pen icon". Check/uncheck to render/un-render the element in the scene. Even maybe with a three state: all rendered, not rendered, partially rendered (children).
  • Add an element color in the tree view. For example I want particles to be in red, tracks in violet and clusters in blue in the tree view

Event scene

  • Can't interact with drawn geometry in eve7 (picking, highlighting, etc ...)
  • Context menu:
    • "Reveal in tree view": Shows the element in the left menu
  • Select a point/object to center on for the camera

C++ interface

  • The TEveBoxSet class was really useful and was lost in translation (maybe on purpose?). It allowed us for example to set different hit size and colors for a cluster having hits in the ECAL and the HCAL.
    • Will be added soon again
  • I found the C++ interface a bit old style and confusing.
    • The memory management is not really clear. I see some RefCount methods dandling around, some pointers passed in methods that doesn't require a pointer to be passed, a (const) reference sometimes being more than enough.
    • A lot of ROOT old style inheritance from multiple classes make it really hard to address my implementation.
    • REveElement is a HUUUUUGE class like some other ones and the interface can be significantly reduced by using composition and "separation of concerns" principle.
    • Again separation would make the interfaces more clear. I found that the objects data (tracks, point sets, etc...) are not enough decoupled from the rest of the framework. A bit like TH1 and TCanvas with the Draw method

3. Nice features

Event scene

  • Navigation with the mouse is nice

4. Questions / HOWTO

@rete
Copy link
Owner Author

rete commented Nov 18, 2019

LCEventDisplay_PandoraPFOs

@linev
Copy link

linev commented Feb 3, 2020

Here is first PR solving remaining "bugs"

root-project/root#4918

It should be possible already now work without internet connection.
We could make round of discussion that should be fixed/improved next

@rete
Copy link
Owner Author

rete commented Feb 3, 2020

Very nice ! This is very good news ! After merging I will have a try.
Thanks Sergey

@linev
Copy link

linev commented Feb 6, 2020

eve7now

I compress m.TreeStandardItem and provide checkbox and color indication.

Still in progress here:
https://github.com/linev/root/tree/eve7_nextimpr

@linev
Copy link

linev commented Feb 6, 2020

Add following line to .rootrc to fix http port:

WebGui.HttpPort: 9222
WebGui.HttpMaxAge: 0

@linev
Copy link

linev commented Feb 12, 2020

Indentation in m.Tree cannot be easily changed. openui5 dynamically adjust it depending of number of levels. See https://experience.sap.com/fiori-design-web/tree/#responsiveness-and-adaptiveness
Many other points should be improved/fixed with root-project/root#4972

@rete
Copy link
Owner Author

rete commented Feb 12, 2020

Thank you for this comment.

For the indent, I guess we would have to live with it for the time being.

For the improvements, I actually pulled the ROOT changes in my docker image and had a quick test.
It looks better indeed. There are more functionalities, in particular the context menu in the scene is really convinient.

But now I have a new issue: it seems that the event navigation is broken. When clicking on next or previous, there is no signal sent to the C++ server. It is either not send or not received, I will have to check in the javascript console and web server and make a proper debug.

I have pushed an updated docker image with the broken behavior under the name rete/lceve:nav-issue.

@alja Do you observe the same problem ?

@linev
Copy link

linev commented Feb 12, 2020

But now I have a new issue: it seems that the event navigation is broken. When clicking on next or previous, there is no signal sent to the C++ server.

I did not test it after I merge my and Matevz changes together. Should be elementary failure, which I will fix tomorrow-

@rete
Copy link
Owner Author

rete commented Feb 12, 2020

I suspected that it might be a piece of code broken on my side, but it is actually not it seems.

This is handler I have:

    /// Go to the next event
    nextEvent : function(oEvent) {
      console.log( "nextEvent" );
      this.mgr.SendMIR({
        "mir": "NextEvent()",
        "fElementId": this.eventMgr.fElementId,
        "class":      "lceve::EventNavigator"
      });
    }

The nextEvent message is actually printed in the console on button press but nothing happens.
The issue is either in the sendMIR() function or on the server side.

@linev
Copy link

linev commented Feb 12, 2020

If code this way - it wrong. Now it should be like:

 this.mgr.SendMIR(
    "NextEvent()", this.eventMgr.fElementId, "lceve::EventNavigator"
  );

@rete
Copy link
Owner Author

rete commented Feb 12, 2020

Also doesn't work

@rete
Copy link
Owner Author

rete commented Feb 12, 2020

I've seen naming convention changing for fElementId. I don't remember in which PR. Can this be the problem ?

@linev
Copy link

linev commented Feb 12, 2020

I've seen naming convention changing for fElementId. I don't remember in which PR. Can this be the problem ?

Check actual value of this.eventMgr.fElementId variable. I did not change naming convention - only arguments of SendMIR

@rete
Copy link
Owner Author

rete commented Feb 12, 2020

This was the PR I was talking about:

root-project/root#4810

but it's not the problem.

@linev
Copy link

linev commented Feb 13, 2020

Now offline features works again: https://linev.github.io/eve7/
Not perfect, but highlight, selection, rendering on/off, change color can be used without server.
Of course, not intend to replace server, just to be able test client code.
And of course, for demos.

@alja
Copy link

alja commented Feb 13, 2020

@rete
Interface of SendMIR has changes. I have updated alja /
EveWebApp repo to root masterbranch:

alja/EveWebApp@f9a0b0b#diff-9a7e3525284176cea731b68e11192716

If you have your version of Summary view than you need to access editor with GedController:
alja/EveWebApp@88e619a#diff-1c056134d847a66e7cc485595352a849

@linev
Copy link

linev commented Feb 14, 2020

@rete I just submit PR to fix several small problems in @alja example.
See here: alja/EveWebApp#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants