Skip to content

Conversation

arthursw
Copy link
Contributor

Added bindings to JavascriptEditorViewport to be able to:

  • react to mouse moves and inputs, and keyboard inputs,
  • set the viewport type and the view mode,
  • project/unproject from screen to world.

Also added a hack to be able to simply draw a selection rectangle on the viewport canvas in FJavascriptEditorViewportClient::Draw(FViewport* Viewport, FCanvas* Canvas), with SelectionBeginPoint and SelectionEndPoint. This should be improved, but it would probably mean exposing FCanvas and FCanvasBoxItem to Blueprint/Javascript.

…d inputs, keyboard inputs, to set the viewport type and the view mode, and to project/unproject from screen to world
FCanvasBoxItem Box(Widget->SelectionBeginPoint, Widget->SelectionEndPoint - Widget->SelectionBeginPoint);
Box.SetColor(FColor::White);
Box.LineThickness = 1.0;
Box.Draw(Canvas);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function seems to handle draw behavior for specific use case. I think there should be some functions which exposes FCanvas*Item accessible to BP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I fully agree, I updated my pull request.

I exposed the FCanvas as a UCanvas like in JavascriptEdMode.

For this purpose, I moved GetCanvasByName() from JavascriptEdMode.cpp to JavascriptEditorLibrary.h to be able to access it from JavascriptEdMode.cpp and JavascriptEditorViewport.cpp.

Tell me if there is a better solution.

Copy link
Contributor Author

@arthursw arthursw Sep 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to make some changes... Working on it...

@nakosung
Copy link
Collaborator

It is recommended to hold UObject references by FGCObject. Could you transform your code not to call AddToRoot?

…Viewport::ProjectWorldToScreen/DeprojectSceneToWorld()
@arthursw
Copy link
Contributor Author

Ok, I did something, I'm not sure that it is a good solution though.

I added

UProperty()
UCanvas *Canvas;

to UJavascriptEditorViewport (I moved GetCanvasByName() back to JavascriptEdMode.cpp).

Instead, I could override FEditorViewportClient::AddReferencedObjects() for the FJavascriptEditorViewportClient (in JavascriptEditorViewport.cpp), maybe it is a better solution?

In JavascriptEditorViewport.cpp:

    virtual void AddReferencedObjects(FReferenceCollector& Collector) override
    {
        FEditorViewportClient::AddReferencedObjects(Collector);
        Collector.AddReferencedObject(CanvasObject);     // Add a reference to the UCanvas object
    }

(In the same way, I could also add Collector.AddReferencedObject(CanvasObject); in FJavascriptEdModeInstance::AddReferencedObjects() in JavascriptEdMode.cpp, and remove CanvasObject->AddToRoot() in GetCanvasByName())

I also fixed some bugs in UJavascriptEditorViewport::DeprojectScreenToWorld()and UJavascriptEditorViewport::ProjectWorldToScreen(), and removed FIntRect ViewRect;and FViewMatrices ViewMatrices; from UJavascriptEditorViewport.

@nakosung
Copy link
Collaborator

You may create a private object which inherits from FGCObject and override AddReferencedObjects. In this way, GC ref management can be done in right way. :)

I think uproperty only for gc isn't a good way.

@arthursw
Copy link
Contributor Author

Ok, but why it is better to create a private object which inherits from FGCObject than to override FEditorViewportClient::AddReferencedObjects()? (FEditorViewportClient is a FGCObject)

@arthursw
Copy link
Contributor Author

Is it better now?

@nakosung
Copy link
Collaborator

Thanks, it is looking good to me!

@nakosung nakosung merged commit 4a7d297 into ncsoft:master Sep 16, 2016
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

Successfully merging this pull request may close these issues.

2 participants