-
Notifications
You must be signed in to change notification settings - Fork 232
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
Fix 208, fix vertex click, misc. upgrades #209
base: develop
Are you sure you want to change the base?
Conversation
The interpolation request declares a type "T", which will be used in the getter methods. When "value" is a Vector3, the getters will get Vector3s; when "value" is a float, the getters will get floats. This procedure therefore works with 3D interpolation (e.g. translocation) as well as 1D interpolation (e.g. scale).
Refine paint vertex controls dock panel Fix several bugs with setting viewports to the wrong size on high DPI devices
More accurate documentation of the tangents in the Hermite cubic interpolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great work.
(Un)fortunately been many years since I looked at C++, and nifskope tbf so can only provide cursory input. Only some weird formatting issues and minor comments.
src/data/niftypes.h
Outdated
@@ -1452,6 +1462,7 @@ class Color4 | |||
|
|||
friend class NifIStream; | |||
friend class NifOStream; | |||
friend class ByteColor4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting issue here
if ( Node::SELECTING ) { | ||
if ( scene->selMode & Scene::SelObject ) { | ||
if ( scene->actionMode & Scene::Object ) { | ||
int s_nodeId = ID2COLORKEY( nodeId ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting seems to be all over the shop here.
as linear interpolation, however if their magnitude is not correct, then | ||
temporally there will be a speed up/down, even if the spacial trajectory is | ||
a straight line. | ||
*/ | ||
|
||
// Tangent 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/gl/glcontroller.cpp
Outdated
setting v1's corresponding "Backward" value to 1 and v2's | ||
corresponding "Forward" to 1 results in a linear interpolation. | ||
*/ | ||
Interpolate X, Y, and Z values independently, along a segment between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 for improving docs
src/glview.cpp
Outdated
glDepthFunc( GL_ALWAYS ); | ||
glLineWidth( 2.0f ); | ||
glColor3f( 1.0, 0.0, 0.0 ); | ||
//drawCircle(Vector3(0, 0, 0), Vector3(0,0,1), 0.2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented code.
src/glview.cpp
Outdated
@@ -622,7 +623,7 @@ void GLView::paintGL() | |||
if ( scene->options & Scene::ShowAxes ) { | |||
// Resize viewport to small corner of screen | |||
int axesSize = std::min( width() / 10, 125 ); | |||
glViewport( 0, 0, axesSize, axesSize ); | |||
glViewport( 0, 0, axesSize*devicePixelRatioF(), axesSize*devicePixelRatioF() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting around operands.
src/glview.cpp
Outdated
|
||
if (cfg.vertexPaintSettings.brushMode == PaintBlendMode::BlendNormal) | ||
{ | ||
blend = (cfg.vertexPaintSettings.brushColor * cfg.vertexPaintSettings.brushOpacity) + (base * (Color4() - cfg.vertexPaintSettings.brushOpacity)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(cfg.vertexPaintSettings.brushColor * cfg.vertexPaintSettings.brushOpacity) can be pulled outside of the for loop.
Could also assign cfg.vertexPaintSettings.brushOpacity to a variable as used in a few locations aswell.
src/ui/widgets/vertexpaintwidget.cpp
Outdated
value_.brushSize = (float)ui->brushSize->value(); | ||
|
||
float opacity = (float)ui->opacity->value() / 255.0f; | ||
if (ui->opacityMode->currentIndex() == 0) // Color only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on moving these ints as to const modes rather than magic numbers?
src/ui/widgets/vertexpaintwidget.cpp
Outdated
|
||
void PaintSettingsWidget::showColorPicker() | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything required here?
src/ui/widgets/vertexpaintwidget.cpp
Outdated
int g = colorDoubleToInt(ui->colorG->value()); | ||
int b = colorDoubleToInt(ui->colorB->value()); | ||
int a = colorDoubleToInt(ui->colorA->value()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment here as feel there is something iteresting going on 😵💫
Address minor PR comments
Thanks for the feedback. I've normalized the whitespace in all files touched by this PR to use tabs (whoops), and addressed all your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I need to look through this still, but also will need to compare it against my dev8 branch. |
Fixes #208
Enhances the vertex click operation.
Adds a vertex color painting mode.