ファイルを読み込む関数でfclose()しているのにも関わらず処理が物凄く重たくなる場合の修正方法が知りたい。

提示コードのvoid Stage::LoadStage()関数が原因で処理が物凄く重たくなりまともに動作しなくなるのですがなぜでしょうか？**毎フレーム実行する関数ではないです該当コードは　//マップオブジェクトを設定　の二重for文をコメントアウトすると問題なく動作するのですがこれはどう修正したらいいのでしょうか？

確認、試したこと
中身が空のfor文やその他のループ文はないかどうか？
画像の読み込みを毎フレームやっているかどうか確認

Github: https://github.com/Shigurechan/ActionGame/tree/07b47f5b32f4dfd6f93e70ca4a9609d891e48d9f
使用しているライブラリ: dxlib

cpp
1
2//コンストラクタ
3Game_Scene::Game_Scene(Scene_Type t,Entry *e) : Scene_base(t,e)
4{
5	Owner = e;	//Entry クラス
6
7
8	// #####　スプライトをロード #####
9
10//	int Player_Handle = Owner->LoadSprite("Assets/Player/Player_0.png");	//プレイヤー
11
12	int Player_Handle[8];
13	LoadDivGraph("Sprite_Data/Player.png", 8, 8, 1, CELL, CELL, Player_Handle);	//プレイヤー
14
15	int Enemy_Handle = Owner->LoadSprite("Assets/Enemy.png");					//エネミー		
16
17	int Player_Bullet_Handle = Owner->LoadSprite("Assets/Player_Bullet.png");	//プレイヤーのバレット
18	int Enemy_Bullet_Handle = Owner->LoadSprite("Assets/Enemy_Bullet.png");		//エネミーのバレット
19
20	int Block_Handle = Owner->LoadSprite("Assets/Block.png");	//ブロック	
21	int Brick_Handle = Owner->LoadSprite("Assets/Brick.png");	//レンガ
22	int Shop_Handle = Owner->LoadSprite("Assets/Shop.png");		//ショップ
23	
24	//ヒットエフェクト
25	int Enemy_HitEffect_Handle[3];	//エネミー
26	int Stage_HitEffect_Handle[3];	//ステージ
27	int Player_HitEffect_Handle[3];	//プレイヤー
28	LoadDivGraph("Assets/Enemy_Explosion.png", 3, 3, 1, CELL, CELL, Enemy_HitEffect_Handle);	//エネミー
29	LoadDivGraph("Assets/Stage_Explosion.png", 3, 3, 1, CELL, CELL, Stage_HitEffect_Handle);	//ステージ
30	LoadDivGraph("Assets/Player_Explosion.png", 3, 3, 1, CELL, CELL,Player_HitEffect_Handle);	//プレイヤー
31	
32
33	player = std::make_shared<Player>(Owner,Player_Handle, Player_Bullet_Handle,Enemy_HitEffect_Handle, Stage_HitEffect_Handle);	//プレイヤー
34	stage = std::make_shared<Stage>(Owner,Block_Handle, Brick_Handle, Shop_Handle);													//ステージ
35	shop = std::make_shared<Shop>(Owner);																							//ショップメニュー
36	enemy_mng = std::make_shared<Enemy_Mng>(Enemy_Handle,Enemy_Bullet_Handle,Stage_HitEffect_Handle, Player_HitEffect_Handle);		//エネミー管理
37
38}
39
40//更新
41void Game_Scene::Update()
42{
43
44	stage->setStage(enemy_mng,player);	//ステージを読み込み
45	enemy_mng->setEnemy(stage);			//エネミーを出現
46
47	stage->Scroll(player, enemy_mng);	//画面スクロール
48
49
50	enemy_mng->Update();	//エネミー更新
51	stage->Update();		//ステージ更新
52	player->Update();		//プレイヤー更新
53
54
55
56
57
58	//　#####　当たり判定 #####
59
60	//エネミーとステージ
61	stage->ColEnemy(enemy_mng);			//　
62	stage->ColEnemy_Bullet(enemy_mng);	//
63
64	//プレイヤーとステージ
65	stage->ColPlayer(player);			//
66	stage->ColPlayer_Bullet(player);	//
67
68	//プレイヤーとエネミー
69	player->ColEnemy_Bullet(enemy_mng);		//
70
71	//エネミーとプレイヤー
72	enemy_mng->ColPlayer_Bullet(player);	//
73	//　##### #####	
74
75
76
77}
78
79//描画
80void Game_Scene::Draw()
81{
82	stage->Draw();		//ステージ描画
83	player->Draw();		//プレイヤー描画
84	enemy_mng->Draw();	//エネミー描画
85}
86
87//デストラクタ
88Game_Scene::~Game_Scene()
89{
90
91}
92

cpp
1void Stage::LoadStage()
2{
3
4	mStage.clear();	//ステージをクリア
5
6	// *** ステージロード ***
7	glm::ivec2 StageSize = glm::ivec2(0, 0);	//画面サイズ
8	FILE* fp = NULL;		//ファイルポインタ
9
10	fopen_s(&fp, "Stage_Data/Debug_Stage.bin", "rb");	//読み込みモードでファイルを開く
11
12	if (fp != NULL)
13	{
14		//ステージサイズを読み込む(先頭８バイト)
15		fread(&StageSize.x, sizeof(int), 1, fp);
16		fread(&StageSize.y, sizeof(int), 1, fp);
17
18		for (int y = 0; y < StageSize.y; y++)
19		{
20			std::vector<byte> tmp;
21			for (int x = 0; x < (int)StageSize.x; x++)
22			{
23				byte b = 0;
24				fread(&b, sizeof(byte), 1, fp);
25				tmp.push_back(b);
26
27			}
28
29
30			Stage_Grid->push_back(tmp);
31
32		}
33		fclose(fp);
34
35	}
36	else {
37		printf("ファイルを読み込めません。\n");
38	}
39
40	//マップオブジェクトを設定
41	for (int y = 0; y < StageSize.y; y++)
42	{
43		for (int x = 0; x < StageSize.x; x++)
44		{
45			switch (Stage_Grid->at(y).at(x))
46			{
47					//ブロック
48				case 1:
49				{
50					mStage.push_back(MapChip(StageObjectType::Block, glm::ivec2(x * CELL, y * CELL), BlockSize, Block_sprite));
51				}
52				break;
53
54				//レンガ
55				case 2:
56				{
57					mStage.push_back(MapChip(StageObjectType::Brick, glm::ivec2(x * CELL, y * CELL), BrickSize, Brick_sprite));
58				}
59				break;
60
61				//ショップ
62				case 3:
63				{
64					mStage.push_back(MapChip(StageObjectType::Shop, glm::ivec2(x * CELL, y * CELL), ShopSize, Shop_sprite));
65				}
66				break;
67
68				default:
69				{
70					mStage.push_back(MapChip(StageObjectType::Shop, glm::ivec2(x * CELL, y * CELL), glm::ivec2(0, 0), -1));
71				}
72			}
73		}
74	}
75
76
77	fclose(fp);
78
79}
80

tiitoi

2021/03/11 06:30 編集

デバッグモードは最適化が行われないので、リリースモードに比べて10倍ぐらい遅くなるのはそういうものなのでは？

退会済みユーザー

2021/03/11 06:57

そうなのでしょうか？画像も数枚しか読み込んでないしそんなに不可がかかっているとは思えないのですが？

sazi

2021/03/11 07:30

遅いというのは実行が遅いという事ですか？ビルドが遅いのではないのですか？

退会済みユーザー

2021/03/11 10:25

いや実行が遅いです。処理が遅いのですがフレームレート

退会済みユーザー

2021/03/11 12:45

以前から遅かったのか、急に遅くなったのかどちら？

退会済みユーザー

2021/03/12 04:03

文章とタイトルを変更しました。

退会済みユーザー

2021/03/12 04:29

とりあえずLoadStageのどこが原因かまで絞り込んでみては。

退会済みユーザー

2021/03/12 04:58 編集

StageSizeのxとyって実際どのくらいのサイズです？

fana

2021/03/12 06:28

これ，　処理が重い　というのは，「Stage::LoadStage()の処理に時間がかかる」という意味ですか？それとも「Stage::LoadStage()を１回実行すると，それ以降，プログラムの（Stage::LoadStage()ではない）他の部分の実行速度が遅くなる」といったような意味ですか？

退会済みユーザー

2021/03/12 06:48

StageSizeは100,100です。

dodox86

2021/03/12 08:51 編集

> fread(&b, sizeof(byte), 1, fp); > tmp.push_back(b); 1バイトごとfread()してvector::push_back()。このようなコードでパフォーマンスは大丈夫なのでしょうか。

dodox86

2021/03/12 09:55

1バイトfread+push_backもさることながら既にご回答いただいているように後段のループのMapChipのコンストラクト+push_backがすごいので、それぞれのループの前後ででも経過時間を計測してみれば、少なくともどこで時間的なコストがかかっているか容易に分かるのでは。

退会済みユーザー

2021/03/14 04:27

原因を絞り込みました。イテレーターによる1万回ループが原因なのですが100x100のステージを全部ループするのは無理なのでしょうか？

行動規範の内容に同意します

回答6件

MapChipを配置しているのだから、その分処理が行われているだけじゃないでしょうか。
当たり判定の処理を全要素対象に回しているなど非効率なので、その分処理に時間がかかっているように思えます。
（なので、Game_Scene::Update()の当たり判定を削れば早くなる）

あと、printf()でデバッグ目的であれば意味のない文字はやめたほうがいいとは思います。

質問者の自己解決が解決になっていないため、追記

Releaseビルドでは高速に処理されたため、少し調べ改造してみました。

・当たり判定とスクロール内のループを範囲for文に変更
・スクロール処理のエネミー周りのループ統合
・Stage::ColEnemyの無駄なコピーの削除
・Box_Collision::Intersectの関数呼び出しを1度ずつにしてローカル変数で処理するように変更
・ColEnemy、ColEnemy_Bullet、ColPlayer、ColEnemy_Bulletを統合してループ数の削減

パッと見える範囲で直してみただけでもかなり改善しますね。
まだまだ無駄なコピーや関数呼び出しが多いので、減らすだけでも改善できそうですね。

ただ、Releaseビルドで高速に動作するのだから動きはそっちで確認すればいいと思いますが。

投稿2021/03/12 09:31

編集2021/03/19 13:14

退会済みユーザー

総合スコア0

退会済みユーザー

2021/03/13 04:18 編集

誤投稿でした。

退会済みユーザー

2021/03/15 09:57

ソースコードを触ってみての感想です。・Entry周りの循環参照がつらい・PlayerとEnemy_Mngの相互参照もつらい・その場しのぎっぽいやけくそ前方宣言がつらい・ifの書き方が混在していてつらい・命名も規則性がなくてつらい・スマートポインタと生ポインタが混在していてつらい・git使ってるのにゴミが残っていてつらい

退会済みユーザー

2021/03/15 10:01

すいませんでした。質問ですがこの場合の命名規則はどうしたらいいのでしょうか？

退会済みユーザー

2021/03/15 15:36

明確なルールがあるわけじゃないので、好きにすればいいです。使った方がいい命名規則があるという話でなく、揃ってないせいで読むのがつらいという話なので。例としてStageクラスのprivateメンバ変数ですが、以下の通りバラバラです。 1. 「mStage」のように、先頭を「m」始まりにしているもの 2. 「mScroll_Vec」のように、先頭を「m」始まり、単語の先頭を大文字かつアンダースコア区切り 3. 「Stage_Grid」のように、単語の先頭を大文字かつアンダースコア区切り 4. 「offsetCol」のように、変数の先頭は小文字、単語の先頭を大文字区切り 5. 「FileNameList」のように、単語の先頭を大文字区切り 6. 「Block_sprite」のように、変数の先頭は大文字、アンダースコア区切り

int32_t

2021/03/15 20:45

C/C++ でメジャーな変数の命名規則は snake_case (ぜんぶ小文字で単語を _ でつなぐ)と camelCase (先頭は小文字、単語の区切り大文字で、あとは小文字)だと思います、クラスのデータメンバの場合は m を付けたり (m_snake_case, mCamelCase)、最後に _ を付けたり (snake_case_, camelCase_)、何も付けなかったり。とにかく、なにかに統一しないとたいへん読みにくくなります。

退会済みユーザー

2021/03/19 12:13

質問ですがfor文とiterator どっちが早いのでしょうか？

退会済みユーザー

2021/03/19 13:16

> 質問ですがfor文とiterator どっちが早いのでしょうか？ for文が添え字によるアクセスであればそちらの方が高速です。また、範囲for文と比較しても、あなたの書いたiteratorは遅いです。 (同じiteratorを使用する形だが、end()の呼び出しやインクリメントで差が出る) ただ、Releaseビルドの最適化で埋まるレベルの差だと思うので読みやすさを優先すればいいかと。 ……速度の話ならなんで自分で計測しないんですか。

退会済みユーザー

2021/03/19 13:26

すいませんでした。

行動規範の内容に同意します

ループ箇所をコメントアウトして軽くなるということであれば、そこでやっている処理が問題です。
MapChipがなにをしているかわかりませんが、この処理が重いのでしょう。
毎回この中で該当オブジェクト(この場合はブロックやレンガ、店などのこと)を生成していませんか？
どこかで予め生成して呼び出すだけにしないと重すぎるのではないでしょうか。

投稿2021/03/12 06:22

ardin

総合スコア544

MapChip の生成・コピーにとてもコストがかかっていませんか?
以前の質問で見たコードのままだと、生成するたびにファイル読み込みしてたと思います。

とりあえず、ループの前に「mStage.reserve(StageSize.x * StageSize.y);」と書くことで MapChipのコピー回数をぐっと減らせるかもしれません。あと、MapChipをコピー不能にしてmStageをスマートポインタのベクタにしたほうが速くなると思います。

投稿2021/03/12 06:05

編集2021/03/12 06:12

int32_t

総合スコア20872

退会済みユーザー

2021/03/14 03:09

スマートポインタにしましたが依然動作重たいです。どうすればいいのでしょうか？Releaseモードだと普通に動作します。

行動規範の内容に同意します

とりあえずfclose(fp);が２か所あるように見えますが，大丈夫なんですか？
（fcloseの多重実施はダメそうに思うが．）

投稿2021/03/12 04:21

fana

総合スコア11656

退会済みユーザー

2021/03/12 04:48

治しましたが以前として重たいです。これはどういったことが原因として挙げられるのでしょうか？空のfor文が回っているととかっていうことなのでしょうか？

行動規範の内容に同意します

StageクラスのLoadStage関数を毎ループ呼び出していませんか？
マップオブジェクトを設定している部分をコメントアウトしたところ、60FPS以上でて安定するようになりました。

投稿2021/03/11 13:41

puuti146

総合スコア11

退会済みユーザー

2021/03/12 03:22 編集

プログラムを修正してLoadStageにprintf();を置いてみたのですが毎フレームは実行されていません。どうすればいいのでしょうか？確かにコメントアウトすると普通に実行できます。またファイルは閉じてます。

puuti146

2021/03/12 11:23

syoshinsya_さんはLoadStage関数は毎ループ実行しない関数と思っていても、毎ループ実行するようにコーディングしてしまえばその挙動になります。他の方の指摘もある通り、LoadStage関数内での処理が負荷になっているものと思われます。

puuti146

2021/03/12 13:10

加えてですが、コンソールを使ってのデバッグ出力をされているようなので、そちらを利用させていただいて確認したところ、敵がステージ上に存在しないかという判定がメインループで常に発生しています。敵が存在しなければ→ステージを再読み込みという仕様にされているようなので、そちらが原因なのでは無いでしょうか？

退会済みユーザー

2021/03/13 04:36

どうやらステージとプレイヤー、エネミー、バレットという当たり判定が重たいみたいなのですがイテレーターを使う以上に早い手段ループはあるのでしょうか？

puuti146

2021/03/13 11:33

ループはイテレータを使っても範囲forであってもwhile文であっても、ループ内で行う処理が重いものでなければ、基本的に速度は同じです。文法ではなく、行っている処理が遅いのだと私は推測します。事実、LoadStage関数の実行をやめると、フレームレートが安定するようになります。つまり、その関数で行っている事が今回syoshinsya_さんが悩んでいる原因となっています。

行動規範の内容に同意します

ベストアンサー

下記の当たり判定のループですがこれはマップのベクター100 x 100つまり1万回毎フレームループしてますが
これではあまりの低速で１フレーム以上掛かっています。これが原因のですが
別の方法を探る必要があるので近いものだけを判定する方法を考えることにしました。

cpp
1
2//プレイヤーのバレットとの当たり判定
3void Stage::ColPlayer_Bullet(std::shared_ptr<Player> player)
4{
5
6
7	int n = GetNowCount();
8
9	for (std::vector<MapChip>::iterator itr = mStage->begin(); itr != mStage->end(); itr++)
10	{
11		for (std::vector<Bullet>::iterator b = player->getBullet()->begin(); b != player->getBullet()->end(); b++)
12		{
13			/*
14			//交差判定
15			if (Box_Collision::Intersect(*itr->mCol,b->mCol) == true)
16			{
17				b->mIsMapHit = true;
18			}
19			*/
20		}
21	}
22
23	printf("time: %d\n", (GetNowCount() - n));
24
25
26
27}