Módulo gráfico: Estou indo no caminho certo?

7

Estou tentando escrever o módulo gráfico do meu mecanismo. Ou seja, esta parte do código fornece apenas uma interface através da qual carregar imagens, fontes, etc. e desenhá-las na tela. Também é um invólucro para a biblioteca que estou usando (SDL neste caso).

Aqui estão as interfaces para os meus Image, Fonte GraphicsRendererclasses. Por favor, diga-me se estou seguindo o caminho certo.

Imagem

class Image
{
  public:
    Image();
    Image(const Image& other);
    Image(const char* file);
    ~Image();

    bool load(const char* file);
    void free();
    bool isLoaded() const;

    Image& operator=(const Image& other);

  private:
    friend class GraphicsRenderer;
    void* data_;
};

Fonte

class Font
{
  public:
    Font();
    Font(const Font& other);
    Font(const char* file, int ptsize);
    ~Font();

    void load(const char* file, int ptsize);
    void free();
    bool isLoaded() const;

    Font& operator=(const Font& other);

  private:
    friend class GraphicsRenderer;
    void* data_;
};

GrapphicsRenderer

class GraphicsRenderer
{
  public:
    static GraphicsRenderer* Instance();

    void blitImage(const Image& img, int x, int y);
    void blitText(const char* string, const Font& font, int x, int y);
    void render();

  protected:
    GraphicsRenderer();
    GraphicsRenderer(const GraphicsRenderer& other);
    GraphicsRenderer& operator=(const GraphicsRenderer& other);
    ~GraphicsRenderer();

  private:
    void* screen_;

    bool initialize();
    void finalize();
};

Editar: algumas alterações no código e mais detalhes.

Por algumas das discussões aqui, decidi substituir meu uso void*por algo assim:

class Image
{
  private:
    struct ImageData;
    std::shared_ptr<ImageData> data_;
};

(Obviamente, farei o mesmo pela Fontturma.)

Também devo mencionar que essas não são minhas aulas finais e completas. Eu só mostro aqui a funcionalidade básica (carregamento e renderização). Em vez de me dizer que funcionalidade você acha que devo adicionar (rotação de imagens, inclinação, redimensionamento, etc.), concentre-se em revisar o que eu já tenho. Vou tentar defender minhas escolhas, ou mudar de abordagem, se não puder.

Paul Manta
fonte
Pergunta: Que tipo de mecanismo é esse? Qual é o escopo esperado do projeto? Estou um pouco preocupado que eu possa estar dando maus conselhos com base em uma concepção errada do que você pretendia que seu projeto fizesse.
21411 ChrisE

Respostas:

1

O que mais me coça é o void *'abuso'.

Ponteiro de vácuo: eu não preciso disso. É uma maneira de limitar o número de arquivos que incluem SDL.h (void * data_ é apenas um SDL_Surface * convertido para void *)

Bem, você pode evitar a inclusão (que eu aprovo), declarando-a em algum lugar conveniente para você.

jv42
fonte
Após alguns comentários, decidi que definiria um privado struct FontDatae terei um std::shared_ptr<FontData>membro como privado em vez do void*. Isso significa que também posso evitar a declaração de encaminhamento, o que significa que não há menção da biblioteca no arquivo de cabeçalho. :)
Paul Manta
Ok, boa solução.
Jv42
6

Sobre interfaces (em geral)

Então, você nos pediu para revisar seus projetos para interfaces.

Você não nos deu interfaces, nos deu declarações de classe completas. Se estas fossem interfaces , eu esperaria ver algo como:

virtual bool load file(const char* file) = 0;

Isso , em C ++, é uma interface. Eu posso substituí-lo em uma subclasse que implementa funcionalidade (de fato, devo!). Se você está escrevendo uma interface, está aplicando a política, e o que precede é como você faz isso.

Metade das reclamações sobre o uso de void * nas outras respostas seria evitada se você tivesse acabado de expor as funções da interface e mantivesse as variáveis ​​membro ocultas (como deveriam ser, em uma classe de interface ).

Rawr.

Em interfaces (sua)

Imagem: Copiando

Você tem um construtor de cópias e um operador igual. O problema que vejo aqui é que não há uma boa maneira de impedir que o usuário faça cópias estranhas e tolas de imagens.

Para você, usando SDL_surfaces, esse é um grande problema . Sem querer ser ofensivo, estou disposto a apostar que você não considerou o que acontece quando libera uma imagem que é uma duplicata de outra imagem. Estou ainda disposto a apostar que você não planejou lidar com cópias profundas e profundas do SDL_surface; portanto, no caso mencionado, você provavelmente liberará uma imagem e suas outras cópias explodirão, matando todos os que você ama .

Solução: SEM CÓPIAS. Não faça isso, não permita. Use uma função de fábrica ou um carregador de estilo C para criar novas instâncias de uma imagem e use-as em vez de permitir cópias ou atribuições iguais. Como alternativa, descubra completamente como copiar uma imagem SDL em profundidade (não muito difícil, mas irritante).

Imagem: manipulação

Como altero suas imagens depois de carregá-las? De acordo com a sua interface, eu não. Ainda assim, você tem certeza de que é uma boa ideia? Como descubro a profundidade de bits de uma imagem? Sua altura? Largura? Espaço colorido?

Fonte

Como faço para desenhar com essa fonte? Como obtenho esse nome? Como evito os problemas de cópia dos quais reclamei acima? Como faço para definir sua cor? Kerning? E o suporte ao Unicode?

Renderer: Geral

Portanto, notei que você tem algumas funções blit * () e também uma função render (). Isso parece implicar que você deseja que os usuários possam enfileirar várias operações de blitting e, em seguida, liberá-las para a tela de uma só vez usando a chamada render ().

Isso é bom; de fato, é assim que a tecnologia de motores do meu grupo também lida com isso. :)

O uso de um singleton aqui é aceitável, principalmente porque você parece querer permitir que o renderizador tenha controle total sobre o desenho das coisas. Se houver apenas uma instância (como provavelmente deveria haver), isso não fará mal a nada. Não é o que fazemos, mas ei, é uma questão de gosto.

Existem alguns grandes problemas que vejo aqui.

Renderer: Transformações

Você parece estar trabalhando apenas em 2D. Isso é bom. MAS...

Como você lida com coisas como girar uma imagem quando a desenha? Escalando? Você precisa de suporte completo para algo chamado transformações afins . Isso permite que você gire, dimensione, traduza, incline e se mova facilmente de uma maneira bonita.

Isso precisa ser suportado (de alguma forma) para texto e imagens.

Renderer: Coloração e mistura

Quero poder misturar cores nas minhas imagens e definir as cores do meu texto. Você deve expor isso.

Também quero ser capaz de fazer coisas como misturar imagens ao cegar, para que eu possa fazer coisas como fantasmas translúcidos, fumaça ou fogo.

Como salvar o problema

Use SFML . Ele tem um design melhor para, bem, praticamente tudo sobre SDL. Eles já fizeram o que você está tentando fazer aqui. No mínimo, observe como eles especificaram suas interfaces e como eles projetaram sua hierarquia de classes.

Observe também que eles abordam a questão das transformações que apontei anteriormente na classe Drawable. E colorir. E misturando.

Eles têm bons tutoriais e documentação, portanto, pode valer a pena dedicar um pouco a isso e ter uma ideia do que seu código deve conseguir.

Boa sorte!

ChrisE
fonte
11
Existem outras decisões estilísticas que você tomou para executar tarefas (métodos load (), void *, uso de singletons etc.) que, francamente, são completamente ofuscadas pela falta de funcionalidade que a interface proposta expõe. Você não deve perder tempo se preocupando com quais recursos de desenvolvimento de software você deve usar quando a sua engenharia de software aqui está tão malditamente quebrada que faz meus olhos sangrarem.
21411 ChrisE
11
Você está começando bem, só precisa de mais experiência para aprender a identificar as funcionalidades que estão faltando. Confira o SFML, crie alguns aplicativos divertidos e volte a isso quando tiver uma idéia mais clara de quais recursos você precisa expor. Não se preocupe com o quão feio é o encanamento até que você tenha tudo o que precisa.
21411 ChrisE
11
Interface: Sim, é uma interface. É o tipo de interface da API, não o tipo de interface da classe virtual. Ponteiro de vácuo: já resolvido. Veja meus comentários sobre a resposta de jv42. | Cópia de imagem: você faz essa suposição com base em quê? | Manipulação: Muitos detalhes das classes que ainda não foram implementadas ou que considerei apenas uma distração para a questão. Além disso, você parece estar pedindo coisas como rotação de imagem, obter o nome da fonte, etc. Vou implementá-las quando e se necessário, não agora. [continuação]
Paul Manta
11
[cont.] SFML: Na verdade, acabei de descobrir esta biblioteca e gosto muito mais. | Conclusão: O que mostrei aqui é apenas o núcleo (carregamento e renderização). Outros detalhes que considerei sem importância mostrar. Além disso, não começarei a implementar nenhum recurso (rotação, inclinação, redimensionamento), a menos que mais tarde eu precise deles. De qualquer maneira, será fácil fazer isso.
Paul Manta
11
+1 para SFML. Ele vai se livrar da maior parte desse material de interface com sua API. : P
O pato comunista
0

Estou um pouco incomodado com o método "load", que quebra o Princípio de Responsabilidade Única (a propósito, para o Pato Comunista, acho que é por isso que ele está usando um const char *, porque a função de carregamento de imagem SDL, codificada em C, não use std :: string). Não deve ser uma tarefa da classe Image carregar-se, por pelo menos dois motivos:

  • Se você pretende otimizar seu uso de memória, convém ter aulas especializadas para carregar recursos.
  • Você manipulará exceções mais facilmente com uma classe dedicada.
Raveline
fonte
Carregar: Não, deve ser um trabalho da classe Image carregar-se, da mesma maneira que o trabalho do std :: ifstream é carregar-se. Este não é o lugar para o gerenciamento de mem: será feito em uma classe ResourceManager separada. | Cordas: Não é difícil converter std::stringpara char*, de modo que não é a razão. a razão é simplesmente que não std::stringtraria vantagens.
Paul Manta
-1

Singleton = BAD, remova imediatamente. Fontes e imagens não devem ter uma free()função, que deve ser o trabalho do destruidor. Não GraphicsRendererdeve oferecer essas Blitfunções, você deve oferecer classes orientadas a objetos que fornecerão uma posição para cada resultado final e a renderização real gerenciada automaticamente pelo GraphicsRenderer. Finalmente, para encapsulamento, use herança, não PIMPL, e definitivamente não use um vazio *, use um ponteiro opaco fortemente tipado.

Aqui estão alguns trechos do meu próprio design, embora eu tenha usado a alternância em tempo de compilação e não a herança em tempo de execução.

class D3D9Render 
{
public:
    std::shared_ptr<D3D9Font> CreateFont();
};
class D3D9Font 
{
public:
    // PUBLIC INTERFACE ---------------------------------------------------
    std::unique_ptr<D3D9Text> CreateText();

    int Height();
    D3D9Font* Height(char newheight);
    D3D9Font(D3D9Render& ref);

    int Width();
    D3D9Font* Width(char newwidth);
    int Weight();
    D3D9Font* Weight(short newweight);
    bool Italic();
    D3D9Font* Italic(bool newitalic);
    string Font();
    D3D9Font* Font(string str);
    D3D9Font* CommitChanges();
};

class D3D9Text 
{
public:

    // PUBLIC INTERFACE ---------------------------------------------------
    D3D9Text* Text(string str);
    D3D9Text* PositionSizeX(short newx, short newxsize);
    D3D9Text* PositionSizeY(short newy, short newysize);
    D3D9Text* Font(std::shared_ptr<D3D9Font> ref);
    D3D9Text* Colour(unsigned int newcolour);
    string Text();
    int PositionX();
    int PositionY();
    int SizeX();
    int SizeY();
    std::shared_ptr<D3D9Font> Font();
    unsigned int Colour();
    D3D9Text* CommitChanges();
};

Aqui, a memória é gerenciada para você - toda a propriedade é gerenciada por meio de apontador inteligente e a interface é completamente orientada a objetos.

DeadMG
fonte
2
Singleton: singletons não são ruins se forem usados ​​corretamente. Basicamente, o renderizador não tem nenhum estado para falar, então o singleton é bom aqui (é mais complexo do que isso). | Livre: a desalocação é tratada no destruidor. A free()função está lá pelo mesmo motivo que a ifstream::close()função existe. | Blit: Estou tentando separar os dados do mecanismo que os usa. | Sprites: Este é apenas o módulo gráfico. Os sprites devem ser manipulados nos níveis mais altos do mecanismo, de maneira semelhante ao que você descreveu. [continuação]
Paul Manta
[cont.] Herança: Eu não entendo como eu poderia usar herança aqui. | Ponteiro de vácuo: por que não deveria ser usado? Eu pensei que era uma boa maneira de limitar o número de arquivos que têm acesso à biblioteca de terceiros. data_é apenas um SDL_Surface*elenco para void*. Sempre que eu uso, eu o lanço de volta SDL_Surface*. | Blit [continuação]: Outra maneira de pensar sobre isso é que a imagem não se desenha & mdash; o renderizador desenha. A imagem simplesmente existe .
Paul Manta
Obrigado por me mostrar std::shared_ptr, no entanto. Obrigado definitivamente vai ser útil.
Paul Manta
11
Livre : dependendo do sistema, a destruição do objeto e de seus recursos associados (como uma textura) pode precisar ocorrer em diferentes segmentos, daí a necessidade de uma free()função; Além disso, pode-se usar um esquema de alocação personalizado que não chama o destruidor.
Sam Hocevar
11
@DeadMG Também estou confuso sobre como fornecer encapsulamento com herança. Eu sempre imaginei que a melhor maneira de esconder membros privados seria aderir ao ponteiro opaco fortemente tipado. Você poderia elaborar sua sugestão de herança?
michael.bartnett
-1

Primeiras coisas que posso identificar:

  • Singletons são muito, muito ruins, geralmente. Não é 'Hum, só quero um desses?' mas mais 'Hm, mais de um deles interromperá o programa?'.
  • ponteiros void * também são bastante arriscados. Por que você precisa de um? Design ruim em algum lugar.
  • Fonte Imageparece estar intimamente relacionado. Talvez você possa levar algumas das funcionalidades acima na hierarquia para a Renderable.
  • Eu acho que sou eu, mas alguma razão para você estar usando const char*mais std::string?
O Pato Comunista
fonte
Bom local const char*, eu não tinha notado isso.
22411 DeadMG
Ponteiro de vácuo: eu não preciso disso. É uma maneira de limitar o número de arquivos que incluem SDL.h( void* data_é apenas uma SDL_Surface*conversão para void*). | Strings: Tenho algum motivo para não usá-los? Não faço manipulações, só preciso de uma corda. Sou livre para usar std::stringfora dessas aulas, se assim o desejar. | Singletons: Sim, mais de um GR quebrará meu código. O GR inicializa o SDL. Sendo um singleton, não preciso me preocupar quando o SDL for inicializado ou encerrado.
Paul Manta
A vantagem de incluir menos arquivos SDL.hé praticamente zero. De alguma forma, eu não teria um renderizador inicializando SDL. Eu resumiria isso ao mecanismo principal, ou qualquer outra coisa. E se você quisesse outras partes do SDL, exceto gráficos?
The Duck comunista
Outros componentes: eu os inicializaria separadamente, nos componentes que precisam dessas peças. | Inicialização: o mecanismo principal (GameManager, geralmente) não deve se importar com detalhes, como a biblioteca que estou usando. Na minha configuração atual, não preciso me preocupar com a inicialização nem um pouco, tudo acontece automaticamente (e é garantido que isso aconteça exatamente quando eu precisar, nem mais tarde, nem antes). | Cabeçalho: Sim, não há nenhuma vantagem técnica real. Eu acho o código mais limpo quando a biblioteca subjacente não está exposta a mim na interface da classe.
Paul Manta
11
Mais do que isso, os únicos pontos que vejo erradamente no design para o qual você tem motivos. Eu continuo afirmando que singletons não são necessários aqui e que ponteiros nulos devem ser substituídos por SDL_Surface. : P
O Pato Comunista