这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Vtec234
Copy link
Contributor

@Vtec234 Vtec234 commented May 22, 2017

A new rendering engine providing functionality designed for our needs. This is work continued from #287. Related: #34, #153, #286. Please submit contributions to the branch here.

EDIT: This branch is still WIP, but since the diff is getting rather large, it's best to merge it and continue working from there. The renderer.tests.renderer_demo works and displays some static objects.

@TheJJ TheJJ added lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before area: renderer Concerns our graphics renderer labels May 22, 2017
@TheJJ TheJJ mentioned this pull request May 22, 2017
42 tasks
@Vtec234 Vtec234 mentioned this pull request Nov 13, 2017
@Vtec234 Vtec234 force-pushed the new-renderer branch 2 times, most recently from ae41129 to 23650fc Compare November 13, 2017 17:36
@zuntrax zuntrax added the big stuff High-impact changes, mainly foundation work label Jan 21, 2018
@heinezen
Copy link
Member

heinezen commented Feb 2, 2018

Pixel-perfect unit hitbox for unit selection and damage areas

There is an additional invisible selection layer for units in the Genie Engine. This layer is basically an invisible box around the centre of the unit sprite that roughly measures 2 * unit_width and 2 * unit_height. It makes moving units and those who are in a battle much easier to select. This selection layer is only present for moving units that are owned by the player who is making a selection as well as deer and relics. Everything else (all buildings, enemy/friendly units, trees, gold, stone) is selected with the pixel-perfect hitbox.

So the order of preference would be:

  • pixel-perfect selection (own units)
  • "tolerance" selection layer (own units)
  • pixel-perfect selection (everything else)

@TheJJ TheJJ mentioned this pull request Feb 16, 2018
@zuntrax zuntrax mentioned this pull request May 6, 2018
@Vtec234 Vtec234 changed the title [WIP] New renderer, cont. New renderer: initial abstraction implementation Jun 15, 2018
@Vtec234
Copy link
Contributor Author

Vtec234 commented Jun 21, 2018

I've now pushed a commit fixing all code compliance issues (copyright dates, mostly). And since Kevin will be down for a bit, I've tested what I could. At least make checkall passes with no errors on my x86_64 Arch Linux and the renderer.tests.renderer_demo works, so this should be mergeable as the first step towards the new renderer. @TheJJ could you please take a look at the code?

Copy link
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

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

that was about half the lines, more to come :)
if you want, i can fix some of the things myself.

@@ -0,0 +1,149 @@
# Openage graphics
Copy link
Member

Choose a reason for hiding this comment

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

can we move this file to doc/code/renderer/README.md? we didn't mix code and documentation so far.


__TODO name__
`openage::graphics` - the level 2 system

Copy link
Member

Choose a reason for hiding this comment

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

And level 3 is the "presenter", which dispatches events to gui, sound, graphics and receives input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's uncertain yet whether there would be level 2 and 3 or whether we can just have the game rendering code and general presentation code in one unit, which would just be level 2. I think it depends on how much code there will be, so that if it's little we can have one unit, but if there's a lot two levels would be better.

texture_array.cpp
vertex_array.cpp
window.cpp
)
Copy link
Member

Choose a reason for hiding this comment

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

cmake should be indented with tabs

std::make_pair(resources::index_t::U8, GL_UNSIGNED_BYTE),
std::make_pair(resources::index_t::U16, GL_UNSIGNED_SHORT),
std::make_pair(resources::index_t::U32, GL_UNSIGNED_INT)
);
Copy link
Member

Choose a reason for hiding this comment

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

indent with tabs pls, all the above indents are 2 spaces.

gl_render_target_t type;

/// For textures target type, the framebuffer.
std::experimental::optional<GlFramebuffer> framebuffer;
Copy link
Member

Choose a reason for hiding this comment

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

no need for experimental any more

///
/// A shader input mapping is only allowed when there is a single element in `buffers`. In such a case,
/// the vertex inputs are paired with VAO attributes according to the mapping instead of in ascending order.
GlVertexArray(std::vector<std::pair<GlBuffer const&, resources::VertexInputInfo const&>> buffers);
Copy link
Member

Choose a reason for hiding this comment

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

const & for the buffers-vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a vector of pointers, so I think passing by value is fine.


// We need HIGHDPI for eventual support of GUI scaling.
// TODO HIGHDPI fails (requires newer SDL2?)
int32_t window_flags = SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE | SDL_WINDOW_MAXIMIZED;
Copy link
Member

Choose a reason for hiding this comment

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

SDL_WINDOW_ALLOW_HIGHDPI | window should be created in high-DPI mode if supported (>= SDL 2.0.1)

but I don't see what the highdpi mode would do. we'll scale the UI with qt, the window resolution should remain unaffected? (https://wiki.libsdl.org/SDL_GL_GetDrawableSize)
I think we can safely ignore highdpi stuff from SDL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure about this as I've never done high-DPI stuff. The docs seem to say that one can have a larger rendering space than the actual window dimensions with ALLOW_HIGHDPI.


/// The window's OpenGL context. It's optional because it can't be constructed immediately,
/// but after the constructor runs it's guaranteed to be available.
std::experimental::optional<opengl::GlContext> context;
Copy link
Member

Choose a reason for hiding this comment

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

no more experimental::

mesh_data.cpp
shader_source.cpp
texture_data.cpp
texture_info.cpp
Copy link
Member

Choose a reason for hiding this comment

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

tab indent

return this->data;
}

std::experimental::optional<std::vector<uint8_t>> const &MeshData::get_ids() const {
Copy link
Member

Choose a reason for hiding this comment

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

no more std::experimental

@Vtec234
Copy link
Contributor Author

Vtec234 commented Jun 30, 2018

The builds are failing because of missing Vulkan on the test machines.

log::log(MSG(info) << "Saving texture data to " << file);

if (this->info.get_format() != pixel_format::rgba8) {
throw "unimplemented";
Copy link
Member

Choose a reason for hiding this comment

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

pls use our Error

amask = 0xff000000;
#endif

SDL_Surface *surf = SDL_CreateRGBSurfaceFrom(
Copy link
Member

Choose a reason for hiding this comment

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

nit: use a uniqueptr here so that if the write fails/exceptions occur before freesurface, the surface is still free'd

std::make_pair(pixel_format::depth24, 3)
);

size_t pixel_size(pixel_format fmt) {
Copy link
Member

Choose a reason for hiding this comment

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

can be a constexpr function, that should allow more optimizations then

pmat << xScale, 0, 0, 0,
0, 1, 0, 0,
0, 0, 1, 0,
0, 0, 0, 1;
Copy link
Member

Choose a reason for hiding this comment

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

indent

}
id--; //real id is id-1
log::log(INFO << "Object number " << id << " clicked.");
renderer->display_into_data().store(path / "/assets/screen.png");
Copy link
Member

Choose a reason for hiding this comment

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

we should store this to /tmp or somewhere else at a disposable location? otherwise the assets folder will be filled up.
I suspect this is a debugging leftover, though.

#define TEST_DBG(txt) \
printf("before %s\n", txt); \
window.get_context()->check_error(); \
printf("after %s\n", txt);
Copy link
Member

Choose a reason for hiding this comment

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

i think that macro is a leftover.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's for debugging yes, but I'd just leave it because it comes in handy often when debugging the tests.

Texture2dArray(resources::Texture2dInfo);

/// Information about the size, format, etc. of every layer in this array.
resources::Texture2dInfo layer_info;
Copy link
Member

Choose a reason for hiding this comment

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

indent

graphics_device.cpp
loader.cpp
renderer.cpp
windowvk.cpp
Copy link
Member

Choose a reason for hiding this comment

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

indent

return {};
}

return details;
Copy link
Member

Choose a reason for hiding this comment

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

indent 1 tab too much


// TODO reinit swapchain on window resize

auto dir = std::make_shared<util::fslike::Directory>("/home/wojtek/Programming/C++/openage/");
Copy link
Member

Choose a reason for hiding this comment

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

😃
you already create the root directory correctly, you can simply pass it here for access to the shaders.

@TheJJ
Copy link
Member

TheJJ commented Jul 12, 2018

To make Vulkan optional, we can decide between a macro that disables the content of each file, or we don't even add the TU's to libopenage. We'd need the macro anyway in order to disable the codepaths that would select vulkan.

@Vtec234 Vtec234 force-pushed the new-renderer branch 3 times, most recently from e05a6cb to ef622b7 Compare July 21, 2018 11:35
Vtec234 and others added 3 commits July 22, 2018 19:03
Co-authored-by: Erik Griese <erik.griese@student.hpi.uni-potsdam.de>
Co-authored-by: surajhanchinal <surajhanchinal@gmail.com>
Co-authored-by: Jonas Jelten <jj@sft.mx>
@TheJJ TheJJ merged commit 93a17ce into SFTtech:master Jul 22, 2018
@heinezen heinezen mentioned this pull request Jun 24, 2023
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: renderer Concerns our graphics renderer big stuff High-impact changes, mainly foundation work lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants